Skip to content

fix(file-provider): make materialized item reconciliation converge - #10682

Open
potanin wants to merge 2 commits into
nextcloud:masterfrom
potanin:fix/materialized-enumeration-convergence
Open

fix(file-provider): make materialized item reconciliation converge#10682
potanin wants to merge 2 commits into
nextcloud:masterfrom
potanin:fix/materialized-enumeration-convergence

Conversation

@potanin

@potanin potanin commented Aug 26, 2026

Copy link
Copy Markdown

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)

@claucambra claucambra left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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
potanin force-pushed the fix/materialized-enumeration-convergence branch from cc4586b to 934dee8 Compare August 27, 2026 05:27
@potanin

potanin commented Aug 27, 2026

Copy link
Copy Markdown
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!

@Rello Rello added this to the 34.0.4 milestone Aug 27, 2026
@Rello Rello added os: 馃崕 macOS Apple macOS, formerly also known as OS X feature: 馃搧 file provider macOS File Provider Extension, more general also known as virtual file system. labels Aug 27, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Artifact containing the AppImage: nextcloud-appimage-pr-10682.zip

Digest: sha256:ff41783833b3b8d98612d14381caad50f310c76318417dced4219546c57a510f

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature: 馃搧 file provider macOS File Provider Extension, more general also known as virtual file system. os: 馃崕 macOS Apple macOS, formerly also known as OS X

Projects

None yet

3 participants