Skip to content

Commit

Permalink
pageserver: reduce size of delta layer ValueRef (#8401)
Browse files Browse the repository at this point in the history
## Problem

ValueRef is an unnecessarily large structure, because it carries a
cursor. L0 compaction currently instantiates gigabytes of these under
some circumstances.

## Summary of changes

- Carry a ref to the parent layer instead of a cursor, and construct a
cursor on demand.

This reduces RSS high watermark during L0 compaction by about 20%.
  • Loading branch information
jcsp authored Jul 16, 2024
1 parent f4f0869 commit 4a90423
Showing 1 changed file with 13 additions and 9 deletions.
22 changes: 13 additions & 9 deletions pageserver/src/tenant/storage_layer/delta_layer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1180,9 +1180,7 @@ impl DeltaLayerInner {
let delta_key = DeltaKey::from_slice(key);
let val_ref = ValueRef {
blob_ref: BlobRef(value),
reader: BlockCursor::new(crate::tenant::block_io::BlockReaderRef::Adapter(
Adapter(self),
)),
layer: self,
};
let pos = BlobRef(value).pos();
if let Some(last) = all_keys.last_mut() {
Expand Down Expand Up @@ -1426,7 +1424,7 @@ impl DeltaLayerInner {
let keys = self.load_keys(ctx).await?;

async fn dump_blob(val: &ValueRef<'_>, ctx: &RequestContext) -> anyhow::Result<String> {
let buf = val.reader.read_blob(val.blob_ref.pos(), ctx).await?;
let buf = val.load_raw(ctx).await?;
let val = Value::des(&buf)?;
let desc = match val {
Value::Image(img) => {
Expand Down Expand Up @@ -1461,8 +1459,7 @@ impl DeltaLayerInner {
use pageserver_api::key::CHECKPOINT_KEY;
use postgres_ffi::CheckPoint;
if key == CHECKPOINT_KEY {
let buf = val.reader.read_blob(val.blob_ref.pos(), ctx).await?;
let val = Value::des(&buf)?;
let val = val.load(ctx).await?;
match val {
Value::Image(img) => {
let checkpoint = CheckPoint::decode(&img)?;
Expand Down Expand Up @@ -1547,17 +1544,24 @@ pub struct DeltaEntry<'a> {
/// Reference to an on-disk value
pub struct ValueRef<'a> {
blob_ref: BlobRef,
reader: BlockCursor<'a>,
layer: &'a DeltaLayerInner,
}

impl<'a> ValueRef<'a> {
/// Loads the value from disk
pub async fn load(&self, ctx: &RequestContext) -> Result<Value> {
// theoretically we *could* record an access time for each, but it does not really matter
let buf = self.reader.read_blob(self.blob_ref.pos(), ctx).await?;
let buf = self.load_raw(ctx).await?;
let val = Value::des(&buf)?;
Ok(val)
}

async fn load_raw(&self, ctx: &RequestContext) -> Result<Vec<u8>> {
let reader = BlockCursor::new(crate::tenant::block_io::BlockReaderRef::Adapter(Adapter(
self.layer,
)));
let buf = reader.read_blob(self.blob_ref.pos(), ctx).await?;
Ok(buf)
}
}

pub(crate) struct Adapter<T>(T);
Expand Down

1 comment on commit 4a90423

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

3201 tests run: 3068 passed, 0 failed, 133 skipped (full report)


Flaky tests (2)

Postgres 14

  • test_location_conf_churn[3]: release
  • test_subscriber_restart: release

Code coverage* (full report)

  • functions: 32.7% (6985 of 21380 functions)
  • lines: 50.1% (55035 of 109932 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
4a90423 at 2024-07-16T22:07:31.022Z :recycle:

Please sign in to comment.