-
Notifications
You must be signed in to change notification settings - Fork 2.1k
[stable9] Filecache repair step #29045
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
Conversation
|
Do we have console output access for repair steps? I just mapped it over to a OCP\ILogger instance for now |
|
Needs a couple of fixes from #28253 then this should work here too 🎉 |
|
Ok let's see how this goes - I think the tests might need some more tweaking with regards to output / repairstep differences |
|
Ahh yes we dont have |
ebf1d67 to
6e3a825
Compare
|
detached subtree repair step fails on oracle... where it checks if the entry has been reattached |
|
Was missing the last fix from vincent 4c15721 |
|
Let's wait for CI here |
|
Jenkins passed 💪 |
|
@tomneedham is this up to date with the latest changes from the stable10 PR ? before merging stable9 would be good to also have stable9.1 backport and merge that one first |
Whenever a parent-child relationship describes a specific path but the entry's actual path is different, this new repair step will adjust it. In the event where another entry already exists under that name, the latter is deleted and replaced by the above entry. This should help preserve the metadata associated with the original entry.
When fixing failed cross-storage moves in the file cache and using the storage id filter, we filter by target storage for phase 1. However, we also need to fix the source storages in phase 2. To do so, a list of affected source storages is now gathered in phase 1 to be run on phase 2.
This instead of iterating over all storages which is way less efficient due to the 1-N nature of potential failed cross-storage moves that we are repairing. If singleuser mode is enabled and "--all --repair" is passed, all storages will be repaired in bulk (no repair filter). If not, it will fall back to iterating over each storage which is slower.
c1cd9c9 to
0414613
Compare
|
Rebased and added path hash commit for performance |
| # add an extra line when verbose is set to optical separate users | ||
| if ($verbose) {$output->writeln(""); } | ||
| $output->writeln("Starting scan for user $user_count out of $users_total ($user)"); | ||
| $r = $shouldRepairStoragesIndividually ? ' (and repair)' : ''; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't remember these changes, did you add them to master/stable10 as well ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| $qb->execute(); | ||
|
|
||
| $text = "Fixed file cache entry with fileid $fileId, set wrong parent \"$wrongParentId\" to \"$correctParentId\""; | ||
| $out->advance(1, $text); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I didn't know we had the ability to display progress in stable9, good
| } | ||
| } | ||
|
|
||
| class LogOutput { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bäh... in my original backport I just removed progress logging.
PVince81
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Changes look ok, but I feel slightly disturbed by the hard-coded LogOutput class. Maybe just remove all logging ?
The other alternative is to use the event emitter like we do in other repair steps. But that might not work when trying to log to the console when run from the scan command.
|
Weird JS setup failures, same like #29407 (comment) |
|
Will require #29477 for JS tests to pass and maybe a PR resubmit to clean the env. |
|
Resend as #29489 to fix JS env issue |
|
This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
Backport of #28253 for stable9 (ignore the branch name, rebased for stable9)