pageserver: ingest pre-serialized batches of values #9579
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
5354 tests run: 5132 passed, 0 failed, 222 skipped (full report)Flaky tests (2)Postgres 17
Code coverage* (full report)
* collected from Rust tests only The comment gets automatically updated with the latest test results
95df95a at 2024-11-01T19:12:09.642Z :recycle: |
VladLazar
force-pushed
the
vlad/raw-buf-from-decoder
branch
6 times, most recently
from
241f79d to
4f13b27
Compare
VladLazar
changed the title
VladLazar
force-pushed
the
vlad/raw-buf-from-decoder
branch
from
4f13b27 to
940aa58
Compare
Base automatically changed from
vlad/move-decoding-logic-to-decoder-module
to
main
October 31, 2024 10:47
VladLazar
force-pushed
the
vlad/raw-buf-from-decoder
branch
from
940aa58 to
3659ea3
Compare
This will replace inmemory_layer::SerializedBatch and also be present in the wire format between safekeepers and pageservers.
Little refactor to clean-up for the next commit.
This is a bigger bite than I would have liked, but this stuff is tightly coupled. The main goal of this commit is to use the `SerializedValueBatch` in `InterpretedWalRecord`. This requires that `DatadirModification` maintains a `SerializedValueBatch` and extends it as new WAL records come in (to avoid flushing to disk on every record). In turn, this cascaded into a number of modifications to `DatadirModification` 1. Replace `pending_data_pages` and `pending_zero_data_pages` with `pending_data_batch`. 2. Removal of `pending_zero_data_pages` and its cousin `on_wal_record_end` 3. Rename `pending_bytes` to `pending_metadata_bytes` since this is what it tracks now. 4. Adapting of various utility methods like `len`, `approx_pending_bytes` and `has_dirty_data_pages`. Removal of `pending_zero_data_pages` and the optimisation associated with it ((1) and (2)) deserves more detail. Previously all zero data pages went through `pending_zero_data_pages`. We wrote zero data pages when filling gaps caused by relation extension (case A) and when handling special wal records (case B). If it happened that the same WAL record contained a non zero write for an entry in `pending_zero_data_pages` we skipped the zero write. Case A: We handle relation extensions differently now. Once the batch grows large enough, we identify the gaps and fill the gaps when handling the entire wal record. Hence, the optimisation is not required anymore. Case B: When the handling of a special record needs to zero out a key, it just adds that to the current batch. I inspected the code, and I don't think the optimisation kicked in here.
VladLazar
force-pushed
the
vlad/raw-buf-from-decoder
branch
from
3659ea3 to
2e3c703
Compare
|
Later-edit: re-benchmarked everything after I pushed some optimisations. |
jcsp
reviewed
Oct 31, 2024
jcsp
reviewed
Oct 31, 2024
erikgrinaker
approved these changes
Oct 31, 2024
skyzh
approved these changes
Nov 1, 2024
|
Benchmarked things again after I added some optimisations (results here). I think this is good to merge. Pending an okay from John for the |
jcsp
approved these changes
Nov 5, 2024
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.
Problem
#9524 split the decoding and interpretation step from ingestion.
The output of the first phase is a
wal_decoder::models::InterpretedWalRecord. Before this patch set that structcontained a list of
Valueinstances.We wish to lift the decoding and interpretation step to the safekeeper, but it would be nice if the safekeeper
gave us a batch containing the raw data instead of actual values.
Summary of changes
Main goal here is to make
InterpretedWalRecordhold a raw buffer which contains pre-serialized Values.For this we do:
SerializedValueBatchtype. This isinmemory_layer::SerializedBatchwith some extra functionality for extension, observing values for shard 0 and tests.inmemory_layer::SerializedBatchwithSerializedValueBatchDatadirModificationmaintain aSerializedValueBatch.DatadirModificationchangesDatadirModificationnow maintains aSerializedValueBatchand extendsit as new WAL records come in (to avoid flushing to disk on every record).
In turn, this cascaded into a number of modifications to
DatadirModification:pending_data_pagesandpending_zero_data_pageswithpending_data_batch.pending_zero_data_pagesand its cousinon_wal_record_endpending_bytestopending_metadata_bytessince this is whatit tracks now.
len,approx_pending_bytesandhas_dirty_data_pages.Removal of
pending_zero_data_pagesand the optimisation associatedwith it ((1) and (2)) deserves more detail.
Previously all zero data pages went through
pending_zero_data_pages.We wrote zero data pages when filling gaps caused by relation extension
(case A) and when handling special wal records (case B). If it happened
that the same WAL record contained a non zero write for an entry in
pending_zero_data_pageswe skipped the zero write.Case A: We handle this differently now. When ingesting the
SerialiezdValueBatchassociated with one PG WAL record, we identify the gaps and fill the them in one go.
Essentially, we move from a per key process (gaps were filled after each new key),
and replace it with a per record process. Hence, the optimisation is not required
anymore.
Case B: When the handling of a special record needs to zero out a key,
it just adds that to the current batch. I inspected the code, and I
don't think the optimisation kicked in here.
Checklist before requesting a review
Checklist before merging