Opened at 2009-02-16T20:28:36Z
Closed at 2009-04-12T01:57:19Z
#628 closed defect (fixed)
"mtime" and "ctime": I don't think that word means what you think it means.
Reported by: | zooko | Owned by: | zooko |
---|---|---|---|
Priority: | major | Milestone: | 1.4.1 |
Component: | code-dirnodes | Version: | 1.3.0 |
Keywords: | Cc: | tahoe-dev@… | |
Launchpad Bug: |
Description
Tahoe has "mtime" and "ctime" associated with the edges in the filesystem graph (the entries in a directory). Tahoe means by "mtime" the most recent time that the link was changed to point to a new node than it previously pointed to ("modify time"), and by "ctime" the most recent time that the link was created when there was previously no link under that name ("create time"). The traditional unix values "mtime" and "ctime" don't have these semantics. In Unix, mtime is the most recent time that the file contents changed, and ctime is the most recent time that the inode changed, which means that either the file contents changed or else the metadata (such as owner or permission bits) changed.
(It is a common mistake to think that "ctime" in unix means create time.)
The tahoe semantics are certainly more useful for Tahoe than unix semantics would be (since we don't have owners or permission bits, and since "the most recent time this link was created when previously there was no link by that name" is potentially useful). But if we're going to be deploying new semantics, let's deploy them under new names. How about if we just spell it out: "create time" and "modify time". That will also be understood by more people than abbreviations would.
Attachments (6)
Change History (20)
comment:1 Changed at 2009-02-16T22:40:30Z by warner
comment:2 Changed at 2009-02-16T23:36:30Z by terrell
So, there are four semantics in the above discussion - numbering them for ease of future discussion...
1) unix ctime - when inode last changed (contents or metadata (owner/permissions) changed)
2) unix mtime - when file contents last changed
3) tahoe create time - the most recent time that the link was created when there was previously no link under that name
4) tahoe modify time - the most recent time that the link was changed to point to a new node than it previously pointed to
comment:3 Changed at 2009-02-16T23:53:58Z by zooko
Thanks, Terrell. Add to that list:
5) Windows NTFS creation time - named "ctime" -- when the file was originally created (Windows apps usually try to preserve this timestamp when doing transfers, backups/restores, etc.)
6) Mac HFS+ creation time - named "crtime" -- when the filesystem object was originally created (so I think this is supposed to be touched when an object in the local filesystem is created, instead of copied from some other source e.g. during a restore, but I'm not sure.)
7) ZFS creation time - named "crtime" -- not sure what this means
Hm, so if I tahoe cp a file from Windows to Tahoe, then tahoe cp it from Tahoe to Linux, then change its owner (touching ctime), then tahoe cp it back to Tahoe and then back to Windows, then it will have its Windows "creation time" overwritten with the time that I changed the ownership? From the perspective of Windows, this is clearly wrong. I think the source of the problem is that the the Windows API use the word "ctime" to mean file creation time (hm... I wonder if it was because they thought that's what the unix "ctime" had been intended for...) and the implementors of the Python API accidentally put both Unix "ctime" and Windows "ctime" into the same slot in the stat call:
http://docs.python.org/library/os.html#os.stat
(NTFS also stores the POSIX "change time" ctime, but I don't think Python offers a way to access it. Because who cares about the POSIX "change time"?)
I think we should make a general rule that we never look at the st_ctime slot of the stat struct unless we then switch on platform type, or perhaps the even more general rule that we never look at the st_ctime slot of the stat struct. ;-)
references:
http://en.wikipedia.org/wiki/Mac_times http://msdn.microsoft.com/en-us/library/ms724290(VS.85).aspx
comment:4 Changed at 2009-02-25T16:02:22Z by zooko
- Milestone changed from undecided to 1.3.1
- Version changed from 1.2.0 to 1.3.0
This is the sort of thing that I'm motivated to fix and deploy sooner rather than later. (I think my motivation is because people are starting to build on top of tahoe, and someone might start to rely on the naming and semantics of this metadata, so the sooner we change it the less backwards-compatibility problem we have.) So I'm putting this into the "1.3.1" milestone.
Of course, most of what is currently being put into the "1.3.1" milestone isn't actually going to get done in time for a 1.3.1 release, but at least we can see that it is the kind of change that *would* be appropriate for 1.3.1 if someone were to do the work to fix it. :-)
Also, nobody should do this until Brian comments on this ticket about my proposal that we stop reading the ctime from the local filesystem, or that if we do read the ctime from the local filesystem, that we then switch on platform type, and if Windows then put the value we read into a metadata element named "create time", else put the value we read into a metadata element named "change time" ("change time" is, I think, the official expansion of the unix abbreviation "ctime"). Also that we store the "mtime" in an element named "modify time".
comment:5 Changed at 2009-02-26T14:55:10Z by zooko
On the tahoe-dev list, Shawn Willden writes:
"IMO, this is a bug in the Python os.stat and os.lstat implementations. They should not return values with radically different meanings in the same field."
http://allmydata.org/pipermail/tahoe-dev/2009-February/001314.html
I'm inclined to agree.
comment:6 Changed at 2009-04-01T14:21:28Z by zooko
- Owner set to zooko
- Status changed from new to assigned
I think I'll check whether there is a simple hack we can do for tahoe-1.4.0 that will provide better future-compatibility on this issue...
comment:7 Changed at 2009-04-08T02:42:26Z by zooko
Okay, so I don't want people building on top of Tahoe and starting to rely on the current timestamp semantics, and I don't want to keep populating Tahoe filesystems with "ctime" values for which we will never be able to tell whether they were creation times or unix ctimes. Therefore, I've prepared a patch which attempts to clarify things. Unfortunately this patch grew and grew as I worked on it, and it explicitly reverts some behavior that Brian deliberately put into "tahoe backup", and I don't understand how changing that behavior will change "tahoe backup", and it causes the "tahoe backup" unit tests to fail in ways I don't understand. However, I still wonder if some subset of this patch (such as the part that populates the metadata dict with lcrtime and lmotime, without the part that uses those values) might be appropriate for Tahoe-1.4.0, so please read this.
Here's what I've got:
- To the "metadata" dict associated with each link, add "link creation time" -- spelled lcrtime -- and "link modify time" -- spelled lmotime. Link creation time is set whenever a link is added to the directory when there was not previous a link by that name. Link modify time is set whenever an extant link is overwritten by a new link (even if it points to the same file). These are the semantics that Tahoe 1.3.0 assigned to ctime and mtime respectively.
- Leave ctime and mtime unchanged for now for backwards compatibility.
- Since this expands the size of a directory entry, raise the size of the initial read chunk from 2000 bytes to 4000 bytes, so that we still get the first few directory entries in the initial read.
- If you are overwriting a link, and the old link does not have a lcrtime but does have a ctime, then copy its ctime into lcrtime.
- In tahoe ls, look for lcrtime first and if that is not present then look for ctime, likewise look for lmotime first and if that is not present then look for mtime. Note that tahoe ls is then going to present those values in the format that GNU ls users expect to find unix ctime and mtime. Note further that GNU ls users apparently think that unix ctime means something that it doesn't mean in unix, but does mean in Tahoe. I don't know how to untangle this.
- Likewise in directory listings in the WUI, if lcrtime exists then list it (as "lcr:"), else list {{{ctime}}} (as "c:"), and if {{{lmotime}}} exists then list it (as "lmo:"), else list {{{mtime}}} (as "m:").
- In tahoe backup, don't set any of these bits of metadata -- ctime, mtime, lcrtime, lmotime. Within the context of Tahoe, all of these are properties of the links in the Tahoe filesystem, not of the files in a local filesystem, and it doesn't make sense to copy them. Probably what we ought to do is copy the timestamps such as mtime out of the local filesystem into a *different* field of the metadata, perhaps named local_mtime or something, so as not to confuse it with the Tahoe mtime (née lmotime), which is something different. If we are going to use the local filesystem's timestamps for any reason, then if platform is Windows then set local_crtime=s.st_ctime, else set local_ctime=s.st_ctime. This means on Windows the local_ctime field doesn't get set, and on non-Window the local_crtime field doesn't get set.
Anyway, the attached patch just removes the code which reads timestamps from the local filesystem. It also changes the behavior of dirnodes to always update the timestamps even if the caller explicitly passed in timestamps in its metadata. Basically, the ctime, mtime, lcrtime, and lmotime are now considered to be special and to be controlled only by source:src/allmydata/dirnode.py and not by external callers. (Perhaps they should be moved to a different namespace -- there should be a "caller-usable metadata" and a "tahoe filesystem metadata".) This is the part that explicitly reverses Brian's design and that breaks things in ways that I don't understand.
- Open http://bugs.python.org/issue5720 (ctime: I don't think that word means what you think it means.) to trac my proposal to disambiguate and extend the Python stat API.
Changed at 2009-04-08T02:45:45Z by zooko
comment:8 Changed at 2009-04-08T12:23:05Z by terrell
i like this idea - seems to capture all the semantic trickiness uncovered earlier...
separate namespaces seem to be even cleaner (tahoe: and local:)...
that said, even without a separate namespace, consider using link_crtime and local_crtime - instead of lcrtime, as lcrtime introduces yet another 'shortcut' that could be construed as ambiguous, which is somewhat responsible for getting us here in the first place.
further, local_ctime, link_motime, local_mtime
remove the ambiguity!
comment:9 Changed at 2009-04-08T15:13:31Z by zooko
Okay here is a new version of my patch which tries harder to leave the old mtime and ctime alone so that it won't break the tahoe backup command. It sequesters the new timestamps into a special entry of the metadata named __sys, and it spells out their names as linkcrtime and linkmotime. In this patch, the rule of the caller setting metadata is simply that the caller can't touch the __sys entry, but can continue to overwrite the ctime and mtime entries, which I think will make backup pass its unit tests again.
Please review with consideration to whether we could apply a patch like this (possibly even a smaller subset of this) for Tahoe-1.4.0.
Changed at 2009-04-08T15:14:08Z by zooko
comment:10 Changed at 2009-04-08T15:18:53Z by zooko
Oh, I see where this patch is breaking backupdb, by updating the mtime and the ctime even if the caller passed in a metadata argument. The following version of the patch doesn't update the mtime and ctime when the caller has passed in a metadata dict (just like Tahoe 1.3.0), but it still updates the __sys timestamps (which is the semantics that I think I want in the future -- those timestamps are describing what changes were made to the tahoe filesystem itself and have nothing to do with timestamps that were associated with the files in their local filesystem before they were uploaded to a tahoe filesystem).
This finally makes it pass test_dirnode and test_backupdb. Now I have to run off to volunteer to teach arithmetic to second-graders.
Changed at 2009-04-08T15:19:14Z by zooko
comment:11 Changed at 2009-04-09T03:22:41Z by zooko
Okay my most recent patch seems fairly good in the code part, but it changed the tests and broke them. Here's a version of it with the same code as the previous patch, but with tests like current trunk tests (with only one tiny change). This one (unlike all my previous patches on this ticket) passes tests.
Changed at 2009-04-09T03:23:41Z by zooko
comment:12 Changed at 2009-04-09T03:24:40Z by zooko
Whoops, no I mean *this* patch...
Changed at 2009-04-09T03:37:01Z by zooko
comment:13 Changed at 2009-04-09T03:52:44Z by zooko
Here's an added unit test that asserts that the caller of dirnode can't override the special __sys entry in the metadata dict:
Changed at 2009-04-09T03:53:07Z by zooko
comment:14 Changed at 2009-04-12T01:57:19Z by zooko
- Resolution set to fixed
- Status changed from assigned to closed
Applied a patch in 972975369279b790 that implements this. Per discussion with Brian on IRC the new sub-dict which can't be written to through the API is named tahoe instead of __sys.
good points!
In a few places ('tahoe cp', and the recently-added 'tahoe backup' command), the mtime/ctime values are meant to be copied directly from the original filesystem, in which case the semantics are the same, so the metadata key names ought to be the same.
But the code in dirnode.py which adds new "mtime"/"ctime" metadata keys (using the "tahoe semantics" described above) ought to use different names for the keys it creates.