Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pageserver: lift decoding and interpreting of wal into wal_decoder #9487

Closed
wants to merge 16 commits into from

Conversation

VladLazar
Copy link
Contributor

A new type is added to [wal_decoder::models], InterpretedWalRecord. This type contains everything that the pageserver requires in order to ingest a WAL record. The highlights are the metadata_record which is an optional special record type to be handled and blocks which stores key, value pairs to be persisted to storage.

This type is produced by
[wal_decoder::models::InterpretedWalRecord::from_bytes] from a raw PG wal record.

The rest of this commit separates decoding and interpretation of the PG WAL record from its application in [WalIngest::ingest_record].

Problem

Summary of changes

Checklist before requesting a review

  • I have performed a self-review of my code.
  • If it is a core feature, I have added thorough tests.
  • Do we need to implement analytics? if so did you add the relevant metrics to the dashboard?
  • If this PR requires public announcement, mark it with /release-notes label and add several sentences in this section.

Checklist before merging

  • Do not forget to reformat commit message to not include the above checklist

This one is a bit less obvious than the previous ones.

I merged some of the logic that was previously in
`WalIngest::ingest_record` to `WalIngest::ingest_xact_record`.
This is an odd one. It requires the current checkpoint value to decide
what to do. That can't trivially be moved to the SK. It's possible with
protocol change, but deferring decision for now. Hence, send the raw
record and let the pageserver figure it out.
This will give us a nice evolution path when we want to add
new actions for a record.
The goal of this commit is to make it clearer when we are ingesting
the whole record versus when we are ingesting an action for the record.

I also merged the VM bits clearing into one function since they were
exactly the same.
We wish to have high level WAL decoding logic in `wal_decoder::decoder`
module. For this we need the `Value` and `NeonWalRecord` types
accessible there, so:
1. Move `Value` and `NeonWalRecord` to `pageserver_api::value` and
   `pageserver_api::record` respectively. I had to add a testing feature
   to `pageserver_api` to get this working due to `NeonWalRecord` test
   directives.
2. Get rid of `pageserver::repository` (follow up from (1))
3. Move PG specific WAL record types to `postgres_ffi::record`. In
   theory they could live in `wal_decoder`, but it would create a
   circular dependency between `wal_decoder` and `postgres_ffi`.
   Long term it makes sennse for those types to be PG version specific,
   so that will work out nicely.
4. Move higher level WAL record types (to be ingested by pageserver)
   into `wal_decoder::models`
A new type is added to [`wal_decoder::models`], InterpretedWalRecord.
This type contains everything that the pageserver requires in order
to ingest a WAL record. The highlights are the `metadata_record`
which is an optional special record type to be handled and `blocks`
which stores key, value pairs to be persisted to storage.

This type is produced by
[`wal_decoder::models::InterpretedWalRecord::from_bytes`] from a
raw PG wal record.

The rest of this commit separates decoding and interpretation of the
PG WAL record from its application in [`WalIngest::ingest_record`].
Copy link

5202 tests run: 4995 passed, 0 failed, 207 skipped (full report)


Flaky tests (1)

Postgres 15

Code coverage* (full report)

  • functions: 31.4% (7559 of 24098 functions)
  • lines: 49.1% (60540 of 123232 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
d82df3b at 2024-10-23T16:11:04.803Z :recycle:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant