Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: make deleteOldFiles / moveNewVersionInPlace more robust #594

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

joshtrichards
Copy link
Member

@joshtrichards joshtrichards commented Dec 19, 2024

Don't do any write operations on files or direcories within RDI.

This will make the Updater more robust in FreeBSD and NFS environments (possibly others but those are the known ones).

Fixes #158

Avoids the problem that arose in #517 due to #515 by keeping the memory usage minimal by doing all the exclusions first, not directly converting the iterator to an array, and so the the resulting array sizes stay minimal.

Verified to work on FreeBSD+NFS web hosting provided by https://hostpoint.sh

Verified to work with 72,000+ objects in the apps/ folder (600+ directories containing 120 files each) without exceeding a memory_limit of 256M.

What this does:

  • basically what we already do in recursiveDelete()
  • most critical in deleteOldFiles() & moveWithExclusions()

Notes:

  • probably would be wise to do in createBackup() & checkWritePermissions() too, but less critical since deletions/renames aren't taking place in those
  • There are opportunities to do refactoring/streamlining/code clean-up but out of scope to keep this PR focused and easily reviewable.

Refs:

Cc: @caco3

@joshtrichards
Copy link
Member Author

Test failure in master is unrelated. It's due to nextcloud/server#49224 and currently impacting all PRs in this repo.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3. to review bug feature: deletion Step 9 feature: moving Step 10 robustness 💪 Enhancements (and bugs) related to robustness
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Updater fails to complete (step Delete old files) / Could not rmdir
1 participant