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

Updater blocks on special filesystem directory (lost+found) #520

Open
pludi opened this issue Dec 21, 2023 · 4 comments
Open

Updater blocks on special filesystem directory (lost+found) #520

pludi opened this issue Dec 21, 2023 · 4 comments

Comments

@pludi
Copy link

pludi commented Dec 21, 2023

Description

Running the updater on an instance where the data directory is on its own ext4 filesystem fails in Step 1 with the notice

Check for expected files

The following extra files have been found:

  • lost+found

Expected behaviour

lost+found either being automatically ignored or availability of a config option of files/directories to ignore.

Details

  • Nextcloud Server: 27.1.4

The same issue was raised before as #390 where the "solution" was to suggest the user just delete the filesystem-specific directory.

@joshtrichards joshtrichards changed the title Updater blocks on special filesystem directory Updater blocks on special filesystem directory (lost+found) Apr 10, 2024
@joshtrichards
Copy link
Member

The same issue was raised before as #390 where the "solution" was to suggest the user just delete the filesystem-specific directory.

We didn't suggest deleting it (the reporter did though). It was suggested that one use a sub-folder within the mount to avoid the issue.

We could exclude lost+found by default. I'm somewhat in agreement that we should do so.

It will need to be excluded in the same way as #183 and #236. That is, it needs to get added in three places just like in those PRs:

  • getExpectedELementsList()
  • createBackup()
  • RecursiveDirectoryIteratorWithoutData

Feel free to manually patch yours to test.

@joshtrichards
Copy link
Member

Related: #68

@Saruspete
Copy link

Saruspete commented Sep 10, 2024

Hello,

Tried to update from 29.0.4 to 29.0.6 (first update on this new installation) and the issue still exists.
Also, due to a large number of files, the "step 3 Backup" took a lot of time, and the web installer failed on it. The html only showed a generic error, so I hit retry, and got the same generic error.
When looking at the xhr, I saw a message showing the upgrade was in progress.
Waited a bit a hit retry button again, but this time was a 503 (unauthenticated).

So I switched on cli, and it was only then that I had the detail

Info: Pressing Ctrl-C will finish the currently running step and then stops the updater.

[✔] Check for expected files
[✔] Check for write permissions
[✘] Create backup failed
RecursiveDirectoryIterator::__construct( [ ... ] /html/public/updater/../data/lost+found): Failed to open directory: Permission denied

Update failed. To resume or retry just execute the updater again.

I saw a lot of proposals with sudo chown www-data: lost+found, but that's a horrible workaround. The lost+found directory is used by fsck of some fs (ext3, ext4, xfs, bcachefs, f2fs) : when a filesystem is corrupted, the tool will scan for all inode and dentry structures in the superblock and place orphaned or incorrect inodes there (with the name #1234, (1234 being the inode-number).
So the inodes (thus files, folder, device... whatever filesystem object) placed here will belong to root, not have coherent names and many times not have consistant data (as the inode has been damaged, its data-blocks are lost too and data is inconsistant).

In definitive, the lost+found folder should never be scanned.
I tried to search for a double-checking of whether the folder is indeed a legit lost+found by searching the inum. While empirically all my ext4 have the inum 11, that's not hardcoded. xfs dosen't even create it until needed by xfs_repair.

So it's best to simply ignore the lost+found folder if it belongs to root.

@joshtrichards
Copy link
Member

The real underlying cause here is #507. We don't even do anything with the data/ directory in the Updater. And data/ isn't actually backed up in that step (that backup step only covers the existing installation files themselves and contents of config/).

Unfortunately, current implementation still steps through filesystem entries in data/ directory (at least in environments where the data directory is located underneath the installation folder). This I/O occurs even though the updater ends up ignoring everything in the data/ folder.

With smaller data directories, from a performance perspective, this isn't all that noticeable, but that changes as the data directory gets larger or if the filesystem is slower (i.e. HDD, network-based).

Anyhow. The problem described in this issue will go away once we address #507 by no longer traversing data/ at all (since we already know we're excluding it and thus not going to touch anything in it).

@joshtrichards joshtrichards self-assigned this Dec 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants