commitlog: Improve skipping behavior of traversals #1902
Merged
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
Shubham8287
approved these changes
Oct 24, 2024
kim
changed the base branch from
kim/core/reducer-ctx-from-tx-inputs
to
master
The `*_from` style traversals have historically yielded commits or transactions before the given from-offset, leaving it to downstream consumers to handle skipping. While folding handles it internally, this behavior is not great for transaction iterators, due to the statefulness of decoding -- it is usually necessary to call `Decoder::skip_record` until the desired offset is found. We would also yield all commits from the start of the nearest segment boundary, which can be quite confusing when using the commit iterators directly. This patch fixes the situation by: * Setting the desired offset as the inital next offset in the `Commits` iterator, instead of the nearest segment boundary. * Looping instead of recursing in the `Commits` iterator while skipping commits, so we can skip until the initial offset without blowing the stack. * Passing the desired offset to `Commit::into_transactions`, such that `Decoder::skip_record` can be called if the offset doesn't lie on the commit boundary.
kim
force-pushed
the
kim/commitlog/skipperer
branch
from
072b68f to
1213b2a
Compare
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
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.
The
*_fromstyle traversals have historically yielded commits or transactions before the given from-offset, leaving it to downstream consumers to handle skipping.While folding handles it internally, this behavior is not great for transaction iterators, due to the statefulness of decoding -- it is usually necessary to call
Decoder::skip_recorduntil the desired offset is found.We would also yield all commits from the start of the nearest segment boundary, which can be quite confusing when using the commit iterators directly.
This patch fixes the situation by:
Commitsiterator, instead of the nearest segment boundary.Commitsiterator while skipping commits, so we can skip until the initial offset without blowing the stack.Commit::into_transactions, such thatDecoder::skip_recordcan be called if the offset doesn't lie on the commit boundary.Expected complexity level and risk
2.5 -- makes head spin
NOTE: On top of #1901, because tooling work depends on the whole series