refactor(general): add epoch.Manager.MaybeCompactSingleEpoch #3728
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3728 +/- ##
==========================================
+ Coverage 75.86% 77.10% +1.23%
==========================================
Files 470 476 +6
Lines 37301 28837 -8464
==========================================
- Hits 28299 22234 -6065
+ Misses 7071 4678 -2393
+ Partials 1931 1925 -6 ☔ View full report in Codecov by Sentry. |
julio-lopez
force-pushed
the
fix/compact-single-epoch
branch
3 times, most recently
from
cc654d9
to
303da76
Compare
julio-lopez
force-pushed
the
fix/compact-single-epoch
branch
from
303da76
to
077d3ab
Compare
julio-lopez
added a commit
that referenced
this pull request
Mar 19, 2024
Refactoring for the original implementation with intRange and getKeyRange from closed-open ranges [lo, hi) to closed ranges: [lo, hi]. The primary motivation is for consistency with the implementation of epoch.RangeMetadata in the same package, and thus avoid confusion and reduce cognitive load. Changes: - adds a getContiguousKeyRange wrapper that checks for contiguity. - getKeyRange simply returns a range with minimum and maximum values for the keys in the map. - changes the range implementation from closed-open ranges [lo, hi) to closed ranges: [lo, hi] where both lo and hi are included in the range. - Additional unit tests are included. - renames intRange to closedIntRange to reflect new functionality. Ref: - Follow up refactor(general): add epoch.getKeyRange helper #3721 - Needed for refactor(general): add epoch.Manager.MaybeCompactSingleEpoch #3728 - Avoid index (epoch) cleanup and compaction during index reads #3638
julio-lopez
force-pushed
the
fix/compact-single-epoch
branch
from
077d3ab
to
d249ac4
Compare
julio-lopez
commented
Mar 19, 2024
redgoat650
approved these changes
Mar 19, 2024
| return err | ||
| } | ||
|
|
||
| if !cs.isSettledEpochNumber(uncompacted) { |
There was a problem hiding this comment.
So if I'm not mistaken, this has to do with the ordering of compaction vs epoch advancement we discussed, right? I.e. if we only advance epoch after successful compaction we only ever expect compaction-eligible epochs to be within the unsettled range.
There was a problem hiding this comment.
@redgoat650 Yes, it's related. See the check right below.
In short, the current snapshot cs only loads the last 3 uncompacted epochs, that is, the current write epoch, and the previous 2. So, there may be some uncompacted epochs for which the blob metadata is not included in cs. The block below accounts for that and explicitly loads the list of the blobs for the "missing epochs".
julio-lopez
force-pushed
the
fix/compact-single-epoch
branch
from
a796c75
to
432fab0
Compare
6 tasks
julio-lopez
added a commit
that referenced
this pull request
Mar 27, 2024
…3651) Perform index epoch compaction and cleanup during repository maintenance * refactor: rename maintenance task for deleting superseded indexes * maintenance task to cleanup epoch markers * maintenance task to advance write index epoch * maintenance task to compact epoch ranges * quick maintenance task to compact a single (index) epoch * full maintenance task to compact a single (index) epoch Ref: - #3638 - #3639 Followup to: - #3603 - #3645 Related helpers and changes: - #3721 - #3735 - #3709 - #3728 - #3727 - #3726
Lyndon-Li
pushed a commit
to Lyndon-Li/kopia
that referenced
this pull request
Mar 29, 2024
…opia#3651) Perform index epoch compaction and cleanup during repository maintenance * refactor: rename maintenance task for deleting superseded indexes * maintenance task to cleanup epoch markers * maintenance task to advance write index epoch * maintenance task to compact epoch ranges * quick maintenance task to compact a single (index) epoch * full maintenance task to compact a single (index) epoch Ref: - kopia#3638 - kopia#3639 Followup to: - kopia#3603 - kopia#3645 Related helpers and changes: - kopia#3721 - kopia#3735 - kopia#3709 - kopia#3728 - kopia#3727 - kopia#3726
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Subset and dependency of #3651
Depends on #3735
Ref: