tahoe-lafs/trac

"mtime" and "ctime": I don't think that word means what you think it means. #628

New issue

Closed

opened 2009-02-16 20:28:36 +00:00 by zooko · 20 comments

zooko commented

2009-02-16 20:28:36 +00:00

(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.

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.

zooko added the

labels 2009-02-16 20:28:36 +00:00

zooko added this to the undecided milestone 2009-02-16 20:28:36 +00:00

warner commented

2009-02-16 22:40:30 +00:00

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.

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.

terrell commented

2009-02-16 23:36:30 +00:00

Owner

So, there are four semantics in the above discussion - numbering them for ease of future discussion...

unix ctime - when inode last changed (contents or metadata (owner/permissions) changed)
unix mtime - when file contents last changed
tahoe create time - the most recent time that the link was created when there was previously no link under that name
tahoe modify time - the most recent time that the link was changed to point to a new node than it previously pointed to

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

zooko commented

2009-02-16 23:53:58 +00:00

Author

Thanks, Terrell. Add to that list:

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.)
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.)
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

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>

zooko commented

2009-02-25 16:02:22 +00:00

Author

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".

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".

zooko added

v/1.3.0

and removed

v/1.2.0

labels 2009-02-25 16:02:22 +00:00

zooko commented

2009-02-26 14:55:10 +00:00

Author

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.

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.

zooko commented

2009-04-01 14:21:28 +00:00

Author

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...

zooko self-assigned this 2009-04-01 14:21:28 +00:00

zooko commented

2009-04-08 02:42:26 +00:00

Author

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.

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: 1. 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. 2. Leave `ctime` and `mtime` unchanged for now for backwards compatibility. 3. 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. 4. 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`. 5. 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. 6. 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:"). 7. 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. 8. 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.

zooko commented

2009-04-08 02:45:45 +00:00

Author

Attachment patch.txt (17373 bytes) added

**Attachment** patch.txt (17373 bytes) added

patch.txt

17 KiB

terrell commented

2009-04-08 12:23:05 +00:00

Owner

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!

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!

zooko commented

2009-04-08 15:13:31 +00:00

Author

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.

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.

zooko commented

2009-04-08 15:14:08 +00:00

Author

Attachment smaller-timefix-patch.txt (17978 bytes) added

**Attachment** smaller-timefix-patch.txt (17978 bytes) added

smaller-timefix-patch.txt

18 KiB

zooko commented

2009-04-08 15:18:53 +00:00

Author

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.

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.

zooko commented

2009-04-08 15:19:14 +00:00

Author

Attachment smaller-timefix-patch-allow-user-controlled-mtimectime.txt (17904 bytes) added

**Attachment** smaller-timefix-patch-allow-user-controlled-mtimectime.txt (17904 bytes) added

smaller-timefix-patch-allow-user-controlled-mtimectime.txt

18 KiB

zooko commented

2009-04-09 03:22:41 +00:00

Author

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.

zooko commented

2009-04-09 03:23:41 +00:00

Author

Attachment smaller-timefix-allow-user-controlled-metadata-and-dont-break-tests.patch.txt (14038 bytes) added

**Attachment** smaller-timefix-allow-user-controlled-metadata-and-dont-break-tests.patch.txt (14038 bytes) added

smaller-timefix-allow-user-controlled-metadata-and-dont-break-tests.patch.txt

14 KiB

zooko commented

2009-04-09 03:24:40 +00:00

Author

Whoops, no I mean this patch...

Whoops, no I mean *this* patch...

zooko commented

2009-04-09 03:37:01 +00:00

Author

Attachment smaller-timefix-allow-user-controlled-metadata-and-really-dont-break-tests.patch.txt (21644 bytes) added

**Attachment** smaller-timefix-allow-user-controlled-metadata-and-really-dont-break-tests.patch.txt (21644 bytes) added

smaller-timefix-allow-user-controlled-metadata-and-really-dont-break-tests.patch.txt

21 KiB

zooko commented

2009-04-09 03:52:44 +00:00

Author

Here's an added unit test that asserts that the caller of dirnode can't override the special __sys entry in the metadata dict:

Here's an added unit test that asserts that the caller of dirnode can't override the special `__sys` entry in the metadata dict:

zooko commented

2009-04-09 03:53:07 +00:00

Author

Attachment one-more-test.patch.txt (1117 bytes) added

**Attachment** one-more-test.patch.txt (1117 bytes) added

one-more-test.patch.txt

1.1 KiB

zooko commented

2009-04-12 01:57:19 +00:00

Author

Applied a patch in changeset: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.

Applied a patch in changeset: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`.