NTFS deduplicated files changed from regular to symlink in go 1.21

[Simon Frei]

Hi,

We got a report in syncthing that some files fail to sync on an ntfs filesystem with deduplication enabled after upgrading the app: https://github.com/syncthing/syncthing/issues/9120
The only change relevant to filesystem handling there was going from go1.20 to go1.21. Debug logging shows that `IsRegular` returns false on `LStat` result of this file, while this is a regular (though possibly deduplicated) file so should be true and was true before.

This change in go 1.21 seems like it's related, as it deals with reparse points:
https://github.com/golang/go/commit/3e44b7d07a7b3c6233eb1bf4cf3cb00a0b85adec
And so does ntfs deduplication according to this doc:
https://learn.microsoft.com/en-us/windows-server/storage/data-deduplication/understand

Now my question is if this was an intentional change, or if this is a regression and should be filed as a bug. The commit message suggest it might more likely be a regression, as it seems focused on performance and mentions symlinks being rare, so I doubt ntfs deduplication was on the radar.

Any comments about this, specifically if you feel like this might be a regression/should be filed as a bug. I haven't tried to repro, don't have a windows ntfs setup handy, but the issue seems clear enough to start reason about it.

Cheers,
Simon

[Ian Lance Taylor]

[ + Quin Muntal ]

> --
> You received this message because you are subscribed to the Google Groups "golang-nuts" group.
> To unsubscribe from this group and stop receiving emails from it, send an email to golang-nuts...@googlegroups.com.
> To view this discussion on the web visit https://groups.google.com/d/msgid/golang-nuts/c7b9a889-b659-4022-a5f1-439bde7e7b8fn%40googlegroups.com.

[Simon Frei]

Our resident historian Jakob found that this isn't the first time we are running into ntfs deduplication issues, that already happened back in 2015 and might have relevant context around the question of "what's the expected behaviour for go std lib here":
Go bug (resolved in 2017): https://github.com/golang/go/issues/10935

[Quim Muntal]

Thanks for reporting Simon.

That change was intentional, intended to fix fix for https://github.com/golang/go/issues/42919. The commit description mentions the performance implications of the changes because it requires one more syscall to stating symlinks.

Setting the ModeIrregular bit for non-symlink reparse points seems like the right option for the general case, working with them normally needs knowledge that the standard library doesn't have.

Having said this, I've done a quick web search looking what other frameworks and applications do with IO_REPARSE_TAG_DEDUP, and the quorum seems to be that it can be treated as a regular file, Windows handles it transparently without user intervention.

For example, boost recently started treating it as a regular file: Treat dedup files as regular files on Windows. · boostorg/filesystem@141727b (github.com).

Please file an issue proposing to special-case IO_REPARSE_TAG_DEDUP as a regular file. We can continue the discussion there. Meanwhile, or if the proposal is not approved, you can still manually get the reparse tag for all non-regular files (e.g. src/os/types_windows.go#L51-L62), and special case whatever reparse tags you need.

Quim

El dia dijous, 21 de setembre de 2023 a les 23:44:59 UTC+2, Ian Lance Taylor va escriure:

[Simon Frei]

Thanks for the followups - I am out this week, I'll followup further (create the ticket) next week, unless someone wants to do so already now (they'd be very welcome to :) ).

In my opinion it's not just a question of what others do (though that seems to align), but also what go did previously: Given neither the commit message nor release notes mentioned an intentional change in behaviour (non-symlink reparse points: <1.21: regular, >=1.21: symlink) the change seems to be against the usual spirit of Go, where existing behaviour is kept intact as much as reasonably possible. I am not complaining, just trying to substantiate that I don't think this is a case of "improving go's handling of these reparse points", but restoring the previous (incidentally better) behaviour of go.