[v23.1.x] storage: use async serialization for large kvstore snapshot batches #11798
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
(cherry picked from commit 8f1b095)
This is useful in kvstore, which composes very large batches for its snapshots, and can cause ~30ms reactor stalls encoding them. (cherry picked from commit e759243)
Direct equivalent of the regular from_iobuf, but it returns a future and calls async_adl instead of adl. (cherry picked from commit b697d9e)
kvstore creates very large batches when the number of partitions per core is high, and doing the encode/decode synchronously can block the reactor for tens of milliseconds. (cherry picked from commit 54c0a3f)
(cherry picked from commit fdda19a)
(cherry picked from commit 29c7f1a)
In tests we are seeing some cases where `last_offset` ends up being larger than `base_offset`. This results in trying to reserve a huge number of slots in the hashmap which subsequently leads to crashes (see below). Fix this by simply looking at the record_count in the header. `last_offset` being smaller than `base_offset` is a result of `.last_offset_delta` being negative. This looks valid to me as the `record_batch_builder` initiliazes it to: ``` .last_offset_delta = static_cast<int32_t>(_records.size() - 1), ``` so it looks like a valid case for an empty batch. Please correct me if this doesn't sound right. Internally the absl hash map implementation rounds user given sizes up to power of twos and also merges its control and element array. For values close to `size_t` this doesn't work out anymore because of overflow and then results in allocating a completely different number (very small) compared to what it then tries to memset. Fixes redpanda-data#10788 (cherry picked from commit 65024e6)
StephanDollberg
approved these changes
Jun 30, 2023
|
Any of the conflicts one needs to pay attention to or was it just small stuff? |
jcsp
approved these changes
Jun 30, 2023
Small stuff. Some argument reordering/tweaking and some new functionality for gc which I didn't add here. |
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.
Backports:
Backports Required
Release Notes
Improvements