From 7debd6162cf805ae5e6ede88d3a26b7b7459c6bf Mon Sep 17 00:00:00 2001 From: Vlad Lazar Date: Mon, 4 Mar 2024 10:45:46 +0000 Subject: [PATCH] pageserver: fix vectored delta layer index traversal There were two issues with it. 1. The `key >= range.start && lsn >= lsn_range.start` was too aggressive. Lsns are not monotonically increasing in the delta layer index (keys are though), so we cannot assert on them. 2. Lsns greater or equal to `lsn_range.end` were not skipped. This caused the query to consider records newer than the request Lsn. --- pageserver/src/tenant/storage_layer/delta_layer.rs | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 5eaf1cc1ce..74c9bddfcd 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -892,15 +892,22 @@ impl DeltaLayerInner { let lsn = DeltaKey::extract_lsn_from_buf(raw_key); let blob_ref = BlobRef(value); - assert!(key >= range.start && lsn >= lsn_range.start); + // Lsns are not monotonically increasing, so we don't assert on them. + assert!(key >= range.start); - let cached_lsn = reconstruct_state.get_cached_lsn(&key); let flag = { - if cached_lsn >= Some(lsn) { + #[allow(clippy::if_same_then_else)] + if lsn >= lsn_range.end || lsn < lsn_range.start { + // If the Lsn is not in the queried range it must be ignored + BlobFlag::Ignore + } else if reconstruct_state.get_cached_lsn(&key) >= Some(lsn) { + // If the Lsn is below the caching line it must be ignored BlobFlag::Ignore } else if blob_ref.will_init() { + // This blob will replace all previous blobs for this key BlobFlag::Replaces } else { + // Usual path: add blob to the read BlobFlag::None } };