Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[🐛 | Bug]: Created datetime is updated on file changes #5694

Closed
mmmarcos opened this issue Nov 2, 2023 · 8 comments
Closed

[🐛 | Bug]: Created datetime is updated on file changes #5694

mmmarcos opened this issue Nov 2, 2023 · 8 comments
Assignees
Labels
C-Bug Issue related to a bug v2 Related to the 2.x branch

Comments

@mmmarcos
Copy link
Contributor

mmmarcos commented Nov 2, 2023

Parsec version tested on:

2.16.0

Platforms tested on:

Linux

Bug description:

The Created datetime is always equal to Updated datetime.

When file is updated, both datetimes are updated.

image

Relevant output:

No response

@mmmarcos mmmarcos added the C-Bug Issue related to a bug label Nov 2, 2023
@vxgmichel
Copy link
Contributor

vxgmichel commented Nov 7, 2023

That's expected when the application does not write the file in-place but instead write to a temporary file and then move it to overwrite the original. Many applications such as xed do that to avoid file corruption.

On linux, I can reproduce this behavior with xed outside of parsec. I suspect this is why the nemo file explorer I use only shows the Date Modified column by default.

My opinion is that parsec should probably do the same, i.e only show the Updated/Modified column since creation time is not a reliable information. We should also investigate and see how windows deal with this issue.

What do you think?

@mmmarcos
Copy link
Contributor Author

mmmarcos commented Nov 7, 2023

I understand the write-to-tmp-then-move but, if this is done by Parsec code, there must be a way to avoid updating the creation date (which is stored in the FileManifest I guess?) during the create/write file primitive.

Personally, I do not use creation date that much but it might be useful or important for some users.

Of course if there is a technical constraint (or if it is not that easy to implement), then I agree we should only display the Updated column.

@vxgmichel
Copy link
Contributor

if this is done by Parsec code, there must be a way to avoid updating the creation date (which is stored in the FileManifest I guess?) during the create/write file primitive.

I'm against doing more than what other file systems do. IMO the behavior we implement should be as close as what we expect from ext4.

However there's also something called birth time, btime or crtime depending on the context (as opposed to ctime which is what we're currently using as creation time). It would be interesting to look into it, although it would change the data model if we were to add a 4th date stat field. We would also need to see how this compares the windows stat data model.

@vxgmichel
Copy link
Contributor

I just checked and on windows there are also 4 fields:

    UINT64 CreationTime;
    UINT64 LastAccessTime;
    UINT64 LastWriteTime;
    UINT64 ChangeTime;

So it might make sense to rethink our data model to properly manage those 4 dates instead of created/updated like we do at the moment.

@mmmarcos
Copy link
Contributor Author

mmmarcos commented Nov 9, 2023

I'm against doing more than what other file systems do. IMO the behavior we implement should be as close as what we expect from ext4.

I agree with you, but this IS already supported in both NTFS and ext4 (crtime), isn't it?

So, what I am trying to say is that our Parsec FS should probably handle file operations in a way that creation time (stored in manifest) is preserved during write (or rename or move) so that we can then pass this information to the underlying OS file system.

However, I understand it might not be that easy to implement in our concurrent model (we've discussed it briefly with @touilleMan yesterday).

So it might make sense to rethink our data model to properly manage those 4 dates instead of created/updated like we do at the moment.

Yep. Better to rethink the data model carefully, instead of quickly patching for this bug.

So what do you think we should do with the current column?

  1. Hide it because the information is not yet reliable (although some users may perceive it as a regression?)
  2. Leave the column as "we plan to support creation time in the future"

@vxgmichel
Copy link
Contributor

vxgmichel commented Nov 9, 2023

I agree with you, but this IS already supported in both NTFS and ext4 (crtime), isn't it?

I ran some more tests and after updating a text file with xed in ext4 I get this:

± stat test
  File: test
  Size: 18        	Blocks: 8          IO Block: 4096   regular file
Device: 10303h/66307d	Inode: 16777928    Links: 1
Access: (0664/-rw-rw-r--)  Uid: ( 1000/  vinmic)   Gid: ( 1000/  vinmic)
Access: 2023-11-09 16:57:57.736770701 +0100
Modify: 2023-11-09 16:57:57.740770797 +0100
Change: 2023-11-09 16:57:57.756771179 +0100
 Birth: 2023-11-09 16:57:57.736770701 +0100

So the brith time is not preserved by the write-to-tmp-then-move approach.

I guess we want to run more tests, especially on windows to see if CreationTime is equally unreliable.

@mmmarcos
Copy link
Contributor Author

mmmarcos commented Nov 9, 2023

Yep, what I meant is that ext4 supports the notion of a creation time (the Birth in stat), which is preserved on mv for example.

But on write, it clearly depends on the editor: vim does not seem to preserve it. Neither xed (linuxmint/xed#302). They must be creating a new inode via the write-to-tmp-then-move approach.

On the other hand, nano does preserve it (writes in place, and leaves crtime unchanged).

@mmmarcos mmmarcos added the v2 Related to the 2.x branch label Nov 15, 2024
@mmmarcos
Copy link
Contributor Author

Closing this as won't fix for the v2 branch.

See #8944 for V3.

@mmmarcos mmmarcos closed this as not planned Won't fix, can't repro, duplicate, stale Nov 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C-Bug Issue related to a bug v2 Related to the 2.x branch
Projects
None yet
Development

No branches or pull requests

2 participants