Fix handling of swap files #2624
Conversation
jbearer
requested review from
philippecamacho,
ImJeremyHe,
sveitser,
tbro and
imabdulbasit
as code owners
sequencer/src/persistence/fs.rs
Outdated
| @@ -222,41 +222,36 @@ impl Inner { | |||
| fn collect_garbage( | |||
| &mut self, | |||
| view: ViewNumber, | |||
| intervals: &[RangeInclusive<u64>], | |||
| intervals: &[RangeInclusive<ViewNumber>], | |||
There was a problem hiding this comment.
I can't work out why this function takes a mutable reference to self. Is it to avoid being called in two places at once?
There was a problem hiding this comment.
Yeah, I think that's the idea. It's not mutating the in-memory state that Rust cares about but it's still conceptually a mutable/exclusive function since it's updating shared files and directoreis
sequencer/src/persistence/fs.rs
Outdated
| let view_number = view; | ||
| let prune_view = ViewNumber::new(view.saturating_sub(self.view_retention)); | ||
|
|
||
| let delete_files = |intervals: &[RangeInclusive<ViewNumber>], |
There was a problem hiding this comment.
I think it would be a bit more straightforward to make delete_files a method on Inner. IIUC the only thing it takes from the scope is prune_view.
| continue; | ||
| }; | ||
| if v > view.u64() { | ||
| for (v, path) in view_files(self.decided_leaf_path())? { |
There was a problem hiding this comment.
More descriptive name than v? Looking at this PR diff, or file in general, I think it would be nice to be a bit more explicit with names that describe different kinds of view number. I think we should only use the bare term view if that's not ambiguous and that is no longer the case when we are comparing views.
|
Backport failed for Please cherry-pick the changes locally and resolve any conflicts. git fetch origin release-ignore-swap-files
git worktree add -d .worktree/backport-2624-to-release-ignore-swap-files origin/release-ignore-swap-files
cd .worktree/backport-2624-to-release-ignore-swap-files
git switch --create backport-2624-to-release-ignore-swap-files
git cherry-pick -x 1f7215e06e2d2e98498761f6ce9b8e1ab688a4c2 |
* Fix handling of swap files * Refactor fs garbage collection
Cleans up some mis-handled swap files that I missed in the last fix (#2528). Thanks to NodeGuardians for identifying this.