Efficient merging with empty batches #407
Open
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
This commit makes merging of batches with empty batches efficient. Previously this operation would be handled like a regular merge, by allocating a new batch and merging the input batches into it. However, if one of the batches is empty, there is no need to allocate a new batch, we can simply extend the bounds of the existing non-empty batch accordingly. The implementation of this optimization is complicated by the fact that there are `Batch` implementations for `Rc<Batch>` and `Abomonated<Batch>`. Both of these only provide an immutable reference to the underlying batch, making it impossible to adjust the batch description to extend the batch's bounds. This commit solves this by introducing new `RcBatch` and `AbomonatedBatch` wrappers that, in addition to the `Rc`/`Abomonated` batch, hold their own `Description`s that override the inner batch's description.
This commit makes sure completed merges are logged also for merges that used the empty batch optimization. Also, if we log the dropping of a merged batch, we shouldn't forget to also log the merge event, so consumers don't become confused about the whereabouts of the input batches.
|
Materialize CI is happy with this change. |
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.
This PR makes merging of batches with empty batches efficient. Previously this operation would be handled like a regular merge, by allocating a new batch and merging the input batches into it. However, if one of the batches is empty, there is no need to allocate a new batch, we can simply extend the bounds of the existing non-empty batch accordingly.
The implementation of this optimization is complicated by the fact that there are
Batchimplementations forRc<Batch>andAbomonated<Batch>. Both of these only provide an immutable reference to the underlying batch, making it impossible to adjust the batch description to extend the batch's bounds. This commit solves this by introducing newRcBatchandAbomonatedBatchwrappers that, in addition to theRc/Abomonatedbatch, hold their ownDescriptions that override the inner batch's description.Some adjustments to logging of merge events is also necessary, to account for the new merge path.
Fixes https://github.com/MaterializeInc/database-issues/issues/6368.