fix(posix): back off retries of files that failed to assimilate - #796
Open
NickWalters wants to merge 1 commit into
Open
fix(posix): back off retries of files that failed to assimilate#796NickWalters wants to merge 1 commit into
NickWalters wants to merge 1 commit into
Conversation
NickWalters
force-pushed
the
fix/posix-assimilation-backoff
branch
from
September 10, 2026 15:40
ee0cc56 to
fcd3901
Compare
A file that failed to assimilate after its checksums were computed, e.g. because the service user can't set extended attributes on it, was assimilated again on every directory listing and scan, reading the whole file each time. Remember such failures per path and don't read the file again while it is unchanged (same mtime, size, mode and inode) until a retry delay has passed. The delay starts at a minute and doubles with every failure up to a day. Failures before the checksum step are cheap and still retried right away. The skip returns the last error so logs still say why the file isn't assimilated. Fixes opencloud-eu/opencloud#3498
NickWalters
force-pushed
the
fix/posix-assimilation-backoff
branch
from
September 10, 2026 16:02
fcd3901 to
fc2f433
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes opencloud-eu/opencloud#3498
If a file can't be assimilated for a reason that doesn't fix itself (in my case the opencloud user had no write permission on the files, so setting the
user.oc.*xattrs failed), nothing remembers that it failed. The next directory listing or scan tries again, and every attempt reads the whole file to compute its checksums before it gets to the part that fails. On my setup that came to about 117 MB/s of reads for 46 hours before I tracked it down.With this change
updateFileremembers files that failed after their checksums were computed, keyed by path, in an expirable LRU (golang-lru is already a dependency). While a file's mtime, size, mode and inode stay the same, it isn't read again until a delay has passed. The delay starts at a minute and doubles with each failure, up to 24h. If the file changes, for example someone chmods it or replaces it, it's retried right away. Entries expire after 48h so paths that no longer exist don't pile up.Only failures after the checksum step are remembered. Everything before it is cheap (locking, parent lookups, reading attributes), so those still retry on the next scan or listing like before. That way a file that failed because its parent directory couldn't be assimilated is picked up as soon as the directory is fixed, and a short ID cache outage, which mostly fails at the parent lookup, doesn't leave files waiting on a backoff.
The skip still returns the original error, so the logs keep saying what's actually wrong:
One gap: a
chownor ACL change doesn't touch mtime, size or mode, so a fix made that way is only picked up at the next retry or after a restart. I thought about comparing ctime instead, but ctime also changes whenever some of the xattr writes succeed, so a file where only part of them fail would loop again.For tests, there's a new spec in
tree_non_watching_test.gothat lists a folder containing a read-only file twice and uses the file's access time to check that only the first listing reads it. It fails on main and passes with this change. It skips when running as root, since root can set xattrs on read-only files, and also on file systems that don't update access times. The unit-test pipeline runs as root in the golang image, so I also added whitebox specs for the retry and backoff logic, which do run there.go test -race ./pkg/storage/fs/posix/...passes locally and golangci-lint is clean on the package.Something related that I didn't change here:
updateFilepropagates a file's size to its parents before writing the file's attributes, so every failed attempt inflates the tree sizes. With this change that only happens once per retry, but it's still wrong, so I fixed it separately in #797. The two PRs don't depend on each other and merge cleanly in either order.