fix(file-provider): make materialized item reconciliation converge - #10682
Open
potanin wants to merge 2 commits into
Open
fix(file-provider): make materialized item reconciliation converge#10682potanin wants to merge 2 commits into
potanin wants to merge 2 commits into
Conversation
potanin
requested review from
camilasan,
claucambra,
i2h3,
mgallien and
nilsding
as code owners
August 26, 2026 23:28
claucambra
approved these changes
Aug 27, 2026
claucambra
left a comment
Collaborator
There was a problem hiding this comment.
I have a small nitpick but generally looks good! Thanks for the contribution
MaterializedEnumerationObserver treats every item in the database's materialized set that is absent from the system's enumeration of materialized items as freshly evicted. Since a81b5da, an evicted directory keeps visitedDirectory == true so it stays subscribed to remote change scanning. That same flag is what keeps an item in materialisedItemMetadatas(), so every previously visited directory without local content reappeared as an eviction candidate on every reconciliation pass. Each pass rewrote its metadata row, logged "Updating item state to dataless." and reported it to the completion handler again, so reconciliation never reached a quiescent state. On accounts with many previously browsed directories this produced a continuous re-marking loop scaling with the size of the materialized set (issue nextcloud#10558 reports 81,563 dataless marks covering 17,473 distinct items within 87 minutes in a single extension process lifetime), with sustained CPU load, database growth and starvation of enumeration, fetching and upload work in the extension. Persist and report only actual state transitions: an eviction candidate whose stored flags already match the dataless target state is now skipped entirely. Directories keep their visitedDirectory subscription, preserving the shared mount root behaviour introduced by a81b5da, and a directory carrying a stale downloaded flag is still cleared and reported, but only once. The updated expectation in testMaterialisedObserverWithMixedState encodes the new contract: a visited directory in steady state is not reported as evicted again. The new regression test drives two reconciliation passes over a dataless visited directory, a stale downloaded directory and an evicted file, asserting that the second pass reports no transitions at all. It fails before this change and passes with it. Full package test suite: 347 XCTest and 59 Swift Testing tests pass. The analysis, fix and tests were authored by Claude Fable 5 running in Claude Code, at the request of and operated by the contributor. Closes nextcloud#10558. Assisted-by: ClaudeCode:claude-fable-5 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Signed-off-by: Alex Potanin <potanin@gmail.com>
potanin
force-pushed
the
fix/materialized-enumeration-convergence
branch
from
August 27, 2026 05:27
cc4586b to
934dee8
Compare
Author
|
Offending comment now removed. Also, been running this modified client as dev on my mac and it works fine now for me at least, yay! Hope to see it make it into the release to switch back to the main. Thank you all! |
Contributor
|
Artifact containing the AppImage: nextcloud-appimage-pr-10682.zip Digest: To test this change/fix you can download the above artifact file, unzip it, and run it. Please make sure to quit your existing Nextcloud app and backup your data. |
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.
Hello,
I had a persistent issue with Nextcloud Mac Desktop client for a while where with my large numbers of files (250K files in one account and a whole lot more in others) when switching from classic to File Provide sync it would just sit there for weeks doing nothing but trying to sync! I finally had to resort to Claude (see below what Fable did) to figure out what is going on. The diagnosis of having a FileProvider cause it to forever enumerate seems to fit. I create an issue but since everyone is probably busy, I thought the fix seems simple enough to ask Claude Fable to fix it.
I reviewed the change, and it looks minimal, as it is a workaround around the FileProvider own report of which item was changed or not with a large regression test added to detect it again. I hope this is acceptable and I am trying to fix the client here, not get any credit so please credit Fable instead. ;-)
Hope this helps!
Cheers,
Alex.
P.S. Below is the more detailed technical description which Fable drafted:
MaterializedEnumerationObserver treats every item in the database's materialized set that is absent from the system's enumeration of materialized items as freshly evicted. Since a81b5da, an evicted directory keeps visitedDirectory == true so it stays subscribed to remote change scanning. That same flag is what keeps an item in materialisedItemMetadatas(), so every previously visited directory without local content reappeared as an eviction candidate on every reconciliation pass. Each pass rewrote its metadata row, logged "Updating item state to dataless." and reported it to the completion handler again, so reconciliation never reached a quiescent state.
On accounts with many previously browsed directories this produced a continuous re-marking loop scaling with the size of the materialized set (issue #10558 reports 81,563 dataless marks covering 17,473 distinct items within 87 minutes in a single extension process lifetime), with sustained CPU load, database growth and starvation of enumeration, fetching and upload work in the extension.
Persist and report only actual state transitions: an eviction candidate whose stored flags already match the dataless target state is now skipped entirely. Directories keep their visitedDirectory subscription, preserving the shared mount root behaviour introduced by a81b5da, and a directory carrying a stale downloaded flag is still cleared and reported, but only once.
The updated expectation in testMaterialisedObserverWithMixedState encodes the new contract: a visited directory in steady state is not reported as evicted again. The new regression test drives two reconciliation passes over a dataless visited directory, a stale downloaded directory and an evicted file, asserting that the second pass reports no transitions at all. It fails before this change and passes with it. Full package test suite: 347 XCTest and 59 Swift Testing tests pass.
The analysis, fix and tests were authored by Claude Fable 5 running in Claude Code, at the request of and operated by the contributor.
Closes #10558.
Assisted-by: ClaudeCode:claude-fable-5
Resolves
#
Summary
TODO
Checklist
AI (if applicable)