It is however correct that we expect the 2s windows mechanism you described above to solve what we are seeing here, isn’t it?
Yes, but my claim is that the same could happen on normal linux with fat, so it might not be android only.
Thank you both for your Feedback. I’ll make some tests tonight and catch the Link from my browsers history
https://github.com/syncthing/syncthing/issues/831#issuecomment-58885709 from calmh contains the link to google code I remembered ;). Edit: I think with another one, the links in the ticket are dead sry.
I’m pretty sure I don’t understand the working of this, but why does it matter if the mtime is changed on a file on android if we know the file content hasn’t changed? Do we need the mtime to work out the global version? For syncthing on a local system, non networked, isn’t the only purpose of the mtime delta to flag up a need for a file specific scan? If the mtime is later than the mtime stored in the db, you scan, then possibly use the mtime to work out the global version of the file (but even then, would a global version ever be one where the mtime on the file is less then its time stored in its local db file, if that mtime was then found to be less than all the other stored mtimes, globally?).
With this Android error condition, don’t you have other indicators of whether or not file metadata has changed but file content hasn’t? Don’t you have clear indicators of an error condition?
Before talking to the cluster, I think there are three relevant mtimes:
File mtime # Indicates when content was last changed File mtime in db # When db thinks content was changed. More accurate file mtime Mtime of the file containing the syncthing db # This is either the latest time the file holding # the syncthing db mtime could be written or # 1 second before that.
Is this wrong?
After an android reboot, after starting syncthing, but before a scan, we have:
a) file mtime # Possibly should be 1 second more than its current value. b) file mtime in db # mtime of file with nanosecond granularity c) db file mtime # Time last scan was done possibly 1 second more than its current value. d) file hash in db # content hash
If the file mtime and mtime for the db file (the file that holds the database) differ by more than second, and the db is greater, we can trust the mtime for the file inside the db. If the file mtime and the db mtime are the same, then the file mtime could be 1 second greater and the file might have been changed.
e) After the initial st scan, if the file contents haven't changed with respect to the hash in the db before the scan, then only the mtime has changed (given that no other metadata has changed). Though with this check alone, it might have been changed then reverted by a user. f) The mtime has changed and is negative with respect to that which was stored in the db before the scan. This indicates an error right there. g) If c), the db file (the file that holds the st db) mtime indicates time of last scan (poss + 1 sec), and if that db file mtime is greater than the file mtime and if e) and if f) then don't we know that the file is subject to the bug?
In this instance, can’t we just reset the mtime of the file back to the original db time, and be done with it? Can’t that be done even before any network traffic between nodes on the cluster occur? Is there any other instance where you get a negative delta between db time and file time that isn’t an error?
If the folder isn’t a
receive only folder, the mtimes get updated
anyway, with the ‘taking shortcut’ logic. It’s only when
the folder is receive only that this becomes an issue. What is
the point of preserving an mtime in that instance?
Change the mtime locally before going online if:
You are checking for a file in a receive only folder.
You have a negative delta between mtime in db and mtime of file. The file has gone back in time.
That negative delta is no more than 1 second.
The file hash is the same as that stored in the db before scanning.
The mtime of the file containing the syncthing db is greater by at least 1 second than the mtime of the file.
If all these conditions are met, locally, then can you be said to be trashing a users data or artificially promoting the chance of a file becoming the global version?
Sorry if I’m missing something obvious.
This hideous hack, run in a shared folder on the Linux box, obviates the bug. It just rounds up mtimes to even values:
find . | while read x ; do SECS=$(stat -c %Y "$x") if [ $(echo "$SECS % 2" |bc) -gt 0 ]; then echo "Rounding up mtime for $x" SECS=$(($SECS + 1)) touch -d @$SECS "$x" fi done
I must admit I haven’t fully followed all the conditions you outlined, as there seem to be some incorrect assumptions. From the beginning
We don’t know a priori that the file content has not changed, and that’s the core problem: A file which hasn’t changed will look to Syncthing like it has (new mtime), which means the file gets rehashed and metadata about that exchanged. Even if the latter part is somehow worked around, hashing it pointlessly is already a deficiency. Therefore it does not only affect receive-only folders, it’s just that the problem gets very apparent there.
Also you seem to get the mtime 2s window issue totally - what’s still unclear about the proposed solution: Consider two mtimes equal if they are within a 2s window. There’s nothing more to it then conceptually (of course there is stuff to sort out internally, that’s partially what Audrius and me were discussing).
I’m seeing this behavior (referred from my issue, linked above).
IIUC, mtime was addressed in 1.2.0 releases here,
I am running 1.2.0 on both linux & android …
IS this the same issue, as yet unresolved?
As this thread talks, it cannot be resolved sensibly, so no, it’s not resolved.
It’s probably slighly better than it was before.
What impact would it have on the “to-revert file Count” if we would ignore mtime totally on a receive only folder on android? Will this cause the revert file Count to be Zero, assuming the user does not change a single file on the phone? If that’s the case, I hope it isn’t too much code deviation … I’m thinking of a last-minute before build commit which makes it better on android builds and not requiring a new folder option in config.xml. Totally understand calmh’s point that a lot “official” folder attributes make testing and ensuring reliability for Syncthing (core) a hard task knowing a lot of possible option combinations and use cases.
Ignoring mtime and only checking for a change based on size has other, obvious disadvantages. I think a way forward that solves the problem has already been established: Mtime-windows, as in only conside the mtime changed if the difference is bigger than the window (inspired by e.g. rsync). Details need to be thought about and then ironed out, but at this point the main obstacle is just for someone to sit down and do it.
Can I do some Android tests to help here, I’ve just setup a test device with a 64 gb sdcard . Do we need to determine the correct mtime-window?!
The mtime window is clear in the FAT32 spec, it’s 2 seconds.
@imsodin Do you think this PR is ready to be ran on my test phones , in other words do you think it is in a state you’d expect it to fix the mtime problem? I would make a test APK for @rahrah then and report back myself if it works.
I’ve made some tests based on PR #5852 and posted the results there.
So this is about two issues: A) 2 sec fat32 timestamp tolerance , solved by pr #5852 B) https://forum.xda-developers.com/android/general/guide-timestamp-attributes-correct-t2960935 - fuse problem, solved by google in Android 8+
Then it’s clear what I’ve to do: I need to setup my test scenario with an Android 8+ device (incl. Sdcard) again and grab logs.
It’s not that issue, that issue is known, and addressed by mtime fs. The timestamps randomly moving around after reboot is the issue, and that is not that issue.
I downloaded catfriend1’s latest apk and tested. Sadly, the problem still seems to persist. I updated all the shared dirs, and reverted as necessary. I rebooted the phone. There are 148 files in need of reversion.
Here’s the results for just one file:
fc2d784f3c72f131e39889123c694f7a9a543bb1 ./advanpix/readme.txt ctime: 2019-07-22 20:30:50.000000000 mtime: 2018-06-15 13:45:26.000000000
fc2d784f3c72f131e39889123c694f7a9a543bb1 ./advanpix/readme.txt ctime: 2019-07-03 19:43:44.106688450 +0100 mtime: 2018-06-15 14:45:27.408177594 +0100
The script was:
STAT=$(stat -c "ctime: %z mtime: %y" "$1") SHA1=$(sha1sum "$1") echo -ne "$(echo "$SHA1 $STAT" |tr -d '\n' )\n"
I’m running Android 9 (pie).
After reboot, all my files, on the sdcard, with odd second stamps ( mtime % 2 == 1), get rounded down to even seconds (mtime % 2 == 0). I reported that in the thread, before. There are only integer seconds on the file ystem, of course.
Thanks for all the work on this.
I would verify that the binary version is right by checking of the mtime window setting is visible in advanced config. If it is, try explicitly setting it to 2
Just speculating: Given android does unspeakable things regarding the FAT mtime precision, wouldn’t it be imaginable that it does similarly stupid things regarding this bug: Cache the mtime when running, i.e. above bug is invisible, mtimefs does not do anything, then on reboot that cache is lost and as the actual mtime on FS wasn’t changed, it’s now something completely different.