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

Handle daylight savings time changes with FAT filesystems #9227

Open
calmh opened this issue Nov 15, 2023 · 15 comments
Open

Handle daylight savings time changes with FAT filesystems #9227

calmh opened this issue Nov 15, 2023 · 15 comments
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)

Comments

@calmh
Copy link
Member

calmh commented Nov 15, 2023

FAT filesystems store timestamps in reference to whatever the computer considers local time. This means that when time changes due to daylight savings the timestamps all shift by one hour in some direction, causing files to be rehashed and resynced (at least metadata wise).

Discussion: https://forum.syncthing.net/t/unwanted-full-rescan-resync-sd-card-syncthing-android-after-timezone-daylight-saving-change/21064

It's of course also possible for "local time" to be UTC or another zone that doesn't shift, or the filesystem can be mounted with tz=UTC to store timestamps in UTC. But this is not the most common setup on Windows or Android where FAT is more common.

One solution to all this, implemented by OwnCloud and possibly other sync programs, is to ignore timestamp changes of precisely one hour. We could do the same. We even have a good place to do it -- the mtimefs. We already virtualize the timestamp on top of what's actually on the filesystem, we would more or less just need to accept differences of precisely one hour where we check for equality.

In theory this means some changes might not get picked up, but it will be rare in practice. On filesystems with better than second timestamp precision it would be essentially impossible to trigger by mistake.

@calmh calmh added enhancement New features or improvements of some kind, as opposed to a problem (bug) needs-triage New issues needed to be validated and removed needs-triage New issues needed to be validated labels Nov 15, 2023
@rasa
Copy link
Member

rasa commented Nov 15, 2023

And don't FAT filesystems have only two-second granularity?

@tomasz1986
Copy link
Contributor

I think one potential problem with Android may be that it is impossible to know for sure whether the filesystem is FAT. The current code only checks whether the filesystem is one of the ext family, and then assumes that anything else is FAT. This would mean that on Android, the workaround would need to be applied on the basis of OS-detection alone (except when ext is detected).

@calmh
Copy link
Member Author

calmh commented Nov 15, 2023

And don't FAT filesystems have only two-second granularity?

They do, but I don't think that affects this.

I think one potential problem with Android may be that it is impossible to know for sure whether the filesystem is FAT.

I don't think we have to care. A better filesystem will have better timestamp precision, rendering false positives due to chance exceedingly unlikely, so we can just use the same logic always. (ext3 being the exception I guess, having only one-second precision, it's possible we would miss precisely one file update if it happened precisely 3600 seconds since the previous change, and didn't change file size or anything else we look at.)

@AudriusButkevicius
Copy link
Member

Do we really want to add these hacks to support a 30 year old filesystem?
Sounds like more downsides than upsides (more code, scenario people can be worried about).

Similarly like we kind of don't support old os'es, I don't see why we should add hacks for old filesystem, especially where the "doesn't work" is an event that happens twice a year, in some places, and just burns some cpu cyclea for some time.

@nekr0z
Copy link
Contributor

nekr0z commented Nov 16, 2023

Do we really want to add these hacks to support a 30 year old filesystem?

We do. "Inclusive" is rather high on our Goals list, (unfortunately?)

Sounds like more downsides than upsides (more code, scenario people can be worried about).

Agreed. Unless we can reliably detect that both a) the filesystem is affected (i.e. FAT) and b) timezone is affected (i.e. not UTC or any other TZ with no DST), it's a bad idea to implement this.

That said, the fact that a timezone can be unaffected by DST now but change it the next year (or vice versa), opens a can of worms of the size I wouldn't want to deal with.

Similarly like we kind of don't support old os'es, I don't see why we should add hacks for old filesystem, especially where the "doesn't work" is an event that happens twice a year, in some places, and just burns some cpu cyclea for some time.

I second that.

However, I can envision a change occuring at the same time, and a wrong state winning and propagating due to "wrong" timestamp in a setup where one system has this time-shift issue and another one doesn't…

@bt90
Copy link
Contributor

bt90 commented Nov 16, 2023

I'm with @AudriusButkevicius on this one. The beneficiary of this hack would be Android, and only for SD cards, which are being phased out by most manufacturers.

@AudriusButkevicius
Copy link
Member

I guess my words were wrong. The filesystem is already supported, for some definition of supported, but I think there is already enough code working around the sadness of FAT. Some of it is kind of fundamental for the application to function, but this one is a mild inconvenience at best, and I'm happy for it to stay there to nudge people onto newer better supported things.

@tomasz1986
Copy link
Contributor

tomasz1986 commented Nov 16, 2023

and only for SD cards

Not only for SD cards. FAT is used for all user storage on Android, including internal and external. In other words, this problem basically affects all Syncthing users on Android.

@calmh
Copy link
Member Author

calmh commented Nov 16, 2023

"It's an old filesystem" is true, but doesn't carry much water when it's used as the primary storage in the latest and greatest version of platforms we supposedly support. Unless the point is that Android users should upgrade to iOS, thus avoiding the hassle of both FAT and Syncthing simultaneously.

To me this sounds like a one or two-line hack that will make things better for a non-negligible subset of users, while having no significant real-word downsides.

@bt90
Copy link
Contributor

bt90 commented Nov 16, 2023

and only for SD cards

Not only for SD cards. FAT is used for all user storage on Android, including internal and external. In other words, this problem basically affects all Syncthing users on Android.

Are you sure? mount on my Pixel 4a 5G lists ext4 and f2fs.

@tomasz1986
Copy link
Contributor

tomasz1986 commented Nov 16, 2023

Are you sure? mount on my Pixel 4a 5G lists ext4 and f2fs.

Is this for the /sdcard partition or /data? The latter is usually ext4 (and sometimes f2fs depending on the manufacturer) but /sdcard is what the user has access to and where the user files go, and that I believe is always formatted as vfat. I've only seen it use other filesystems on some custom ROMs, which implemented special tricks to allow this to happen.

Just to avoid confusion, /sdcard despite its name is the internal storage. The name is like that for legacy reasons, because the first Android devices used actual SD cards for their user storage.

@rasa
Copy link
Member

rasa commented Nov 16, 2023

And don't FAT filesystems have only two-second granularity?

They do, but I don't think that affects this.

@calmh Agreed. My thought was we could use gopsutil's PartitionStat.Fstype to see if the mount is a FAT variant. If that fails (or is non-determinitive), we could see if the filesystem supports sub-two-second timestamp granularity.

@er-pa
Copy link
Member

er-pa commented Nov 17, 2023

Are you sure? mount on my Pixel 4a 5G lists ext4 and f2fs.

Is this for the /sdcard partition or /data? The latter is usually ext4 (and sometimes f2fs depending on the manufacturer) but /sdcard is what the user has access to and where the user files go, and that I believe is always formatted as vfat. I've only seen it use other filesystems on some custom ROMs, which implemented special tricks to allow this to happen.

Just to avoid confusion, /sdcard despite its name is the internal storage. The name is like that for legacy reasons, because the first Android devices used actual SD cards for their user storage.

Pretty sure that the whole SDCardFS and /sdcard scheme was deprecated in Android 11+ combined with kernel 5.4+ and no longer exist on any device running the named combination of versions or anything more recent. That being said, the only fat-partitions here are some low-level firmware related things. The rest is either f2fs or ext4, and by the looks of it a FUSE-implementation was used to cover some functionality previously handled by SDCardFS.

calmh added a commit to calmh/syncthing that referenced this issue Nov 17, 2023
@tomasz1986
Copy link
Contributor

Thanks for clarification! My information was a bit outdated then. I would only add that the above seems to only be applicable to devices that came out with Android 11 out of the box. For example, my phone runs Android 11 but was originally released with Android 9, and the filesystem is still vfat.

@JardaG
Copy link

JardaG commented Nov 18, 2023

I'm the author of the original suggestion, I recommend reading the link in the first post as to why this is an annoying problem from my perspective and experience. All my mobile phones have had and will always have some sort of memory card, now 0.5TB (Android 9 + MIUI) and 1TB (Android 13 + MIUI) micro SDXC cards, and I believe mobile phones with SD cards will not die out! I have also used root and am not about to resign to my backup and data workflow needs.

So let's not address that it's an outdated file system, that not enough people use it, that the Android API is bad, that there won't be SD cards in mobile phones, or that people don't travel (see below), thank you.

First, ST rescan on FAT affects not only daylight saving time, but also travel/changes between time zones, and in that case the first solution proposed by calmh is completely unusable, because there are time zones in the world even like UTC+5:30 (India) or UTC+5:45 (Nepal), so purely 1 or X hour offset is not a solution.
https://en.wikipedia.org/wiki/Time_zone

Second, if there is already timestamp virtualization via FAT using mtimefs, then in my opinion it is sufficient to add one correction "constant" to correct the ST and rescan behavior according to the current time offset detection.

Time offset detection methods:

  • by heuristics over a sample of user files to detect that all/most files have the same time offset to mtimefs database,
  • using a special calibration file(s) hidden in the .stfolder so that the time of its creation is known and perhaps confirmed by the user,
  • if ST via the Android API detects a daylight saving time change and/or a move between time zones, stop rescan over FAT storage and prompt the user to set the time compensation correctly. (Even though it's against ST Goal "4. Automatic".)

Maybe you can come up with other algorithms, but I think these three in some combination can provide a high level of prevention of false and unnecessary rescans and unnecessary synchronization of misleading metadata to other clients. Especially when I have hundreds of GB of data on my SD cards.

BTW: If anyone has tried exFAT or some other file format to solve the described problems, I'd be happy to try it for myself (without root for now!).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features or improvements of some kind, as opposed to a problem (bug)
Projects
None yet
Development

No branches or pull requests

8 participants