feat(pageserver): gc-compaction split over LSN

[skyzh]

Problem

part of #9114, stacked PR over #9897, partially refactored to help with #10031

Summary of changes

• gc-compaction takes above_lsn parameter. We only compact the layers above this LSN, and all data below the LSN are treated as if they are on the ancestor branch.
• refactored gc-compaction to take GcCompactJob that describes the rectangular range to be compacted.
• Added unit test for this case.

[github-actions]

7716 tests run: 7397 passed, 0 failed, 319 skipped (full report)

---

Flaky tests (7)

Postgres 17

• test_lr_with_slow_safekeeper: release-arm64
• test_nbtree_pagesplit_cycleid: release-arm64
• test_physical_replication_config_mismatch_too_many_known_xids: release-arm64
• test_check_visibility_map: debug-x86-64, debug-x86-64

Postgres 16

• test_physical_replication_config_mismatch_max_locks_per_transaction: release-arm64
• test_storage_controller_node_deletion[True]: release-x86-64

Code coverage* (full report)

• functions: 31.5% (8362 of 26562 functions)
• lines: 48.0% (66400 of 138398 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
adc8d2a at 2024-12-11T20:10:37.944Z :recycle:}

[problame]

It would make sense to express CompactOptions compact_below_lsn and compact_above_lsn as an unbounded range, instead of as two options.

Maybe fold them into CompactRange, so that CompactRange can represent the entire rectangle.

Please do that in a future PR though.

---

It would be nice to add a test that tests the interaction of key & lsn range limits, i.e., a test that tests behavior of specifying a full rectangle.

Maybe just copy-paste and adjust the test you added in this PR.

---

Definition of compact_above_lsn.
Above is kinda ambiguous wrt inclusivity vs exclusivity.

[skyzh]

It would make sense to express CompactOptions compact_below_lsn and compact_above_lsn as an unbounded range, instead of as two options.

Included in 233eebb, where I did some refactors to make the compaction job desc more clear.

It would be nice to add a test that tests the interaction of key & lsn range limits, i.e., a test that tests behavior of specifying a full rectangle.

part of 20b6336

Definition of compact_above_lsn.

Comments are added in 233eebb. But actually, whether it's exclusive/inclusive doesn't actually matter because the compaction picker will not treat this input range as a definitive input -- it will change the lsn range based on the layer selection to avoid cut a layer in half.

[problame]

I think we need to track 4 rewritten layers now

• above rectangle
• below rectangle
• left of rectangle
• right of rectangle

neon/pageserver/src/tenant/timeline/compaction.rs

Line 2308 in 19809e9

desc.lsn_range.clone(),

Should have a test case for this before fixing.
I think the scenario is

• base image layer
• a 4-key-wide-4 lsn-tall delta layer on top
• a 2-key-wide-2-lsn-tall compact_{key,lsn}_range

[skyzh]

above rectangle
below rectangle

The selection algorithm will not select an LSN that intersects with the layer, so we never need to rewrite those layers (and that's why they didn't get tracked)

[skyzh]

Looking at the current code, the delta layer lsn range is generated as expected.

        let mut delta_layer_writer = SplitDeltaLayerWriter::new(
            self.conf,
            self.timeline_id,
            self.tenant_shard_id,
            lowest_retain_lsn..end_lsn,
            self.get_compaction_target_size(),
        )
        .await?;

Here lowest_retain_lsn = min_layer_lsn, and end_lsn = max_lsn. So the generated delta layer is actually min_layer_lsn..max_layer_lsn. This means that all current tests that checks layer key will ensure delta layers are created using the correct lsn range. We can merge the patch for now.