Conversation
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
Batch the file-cache propagation around the move-to-trash payload and cache mutation, using the propagator batching that the trash expiration path already relies on, so ancestor updates from one folder move are collected and committed together instead of being propagated piecemeal through shared source and trash ancestry. The batch is balanced with try/finally so it is committed on failed moves and on the cleanup paths as well. Fixes nextcloud#44414 Signed-off-by: Matt Van Horn <455140+mvanhorn@users.noreply.github.com>
3fd4d9a to
f360e29
Compare
This branch has not been deployed
2 participants
Summary
Deleting several large folders at the same time can remove every source folder while leaving fewer top-level entries visible in the user's trash bin. The report reproduces this with three folders containing 10,000 files each, and rules out quota-driven trash expiration.
Each
Trashbin::move2trashcall transfers a large cache subtree and immediately propagates changes through shared source and trash ancestors, while the existing destination lock only coordinates collisions for a single generated trash filename. That leaves distinct concurrent folder moves mutating the same file-cache ancestry without the batched propagation that the trash expiration path already uses.This wraps the move-to-trash payload and cache mutation in
beginBatch()/commitBatch()on the trash storage propagator, so the ancestor updates from one folder move are collected and committed together rather than piecemeal. The batch is balanced withtry/finallyso it is committed on failed moves and on the cleanup paths as well. Per-destination locking, metadata insertion, version retention, expiration scheduling, and return behavior are unchanged.The semantic change is six lines; the rest of the diff in
Trashbin.phpis the reindentation that wrapping introduces, sogit diff -wis the easier read.Honest status
I could not run the test suite locally, so the two new tests in
StorageTest.phphave not been executed anywhere yet. CI will be their first run, and I will fix whatever it turns up.The underlying report has no deterministic reproduction, so treat the diagnosis as a hypothesis rather than something I have observed under a debugger. If maintainers who know this code think the batching is the wrong direction, I am happy to close this.
Checklist
3. to review, feature component)stable32)AI (if applicable)