I think it should be enabled by the wrapper for specific folders for the reason the wrapper can ask if there is a sdcard underlaying and go cannot. The option can be hidden or only be available for android builds that would be fine to me. Would love to help from the wrapper side.
Why did you have to mention that explicitly - until now I successfully suppressed that thought and blissfully assumed that this must be an android (only) fuckup xD
It is Android only, found a Link yesterday pointing out it’s on googles bugtracker since 4.4.x (yeah thats long). Hm checked again seems also to depend on mount options in unix.
Another thought: Wait for an incoming file to be written on android fat, store its perceived mtime in a second column of the db aside the expected mtime. If the perceived mtime did not change during next scan, report the expected mtime to the sync cluster devices so they know android still has the right version of the file. E.g. this helps in cases where I download a video with newpipe and the mtime constantly goes up until the file finished writing and then settles to a constant value letting the puller on the other side fetch it successfully (it couldnt complete pulling while the file is in download progress)
Good to know, thanks. Could you share that link for posterity?
And that wouldn’t work, as the cache is only dropped and thus the “real mtime” reported after a phone restart, not on next scan (I am assuming Richards steps above as the canonical description of the problem Ignore Permissions / Different mtimes on Android Sdcard).
The download of a big file problem is a different issue, I’d argue that’s the fault of whatever you use to copy that file. It could use a temp file during transfer or only update mtime once it’s finished. Or you could increase Syncthing’s change watcher delay. Anyway lets keep this discussion focused on the mtime caching problem, if necessary this can discussed in a separate thread.
I don’t think the android bug is related to the caching issue we are seeing here.
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.