feat(pageserver): add fast path for sparse keyspace read

[skyzh]

Problem

In #9441, the tenant has a lot of aux keys spread in multiple aux files. The perf tool shows that a significant amount of time is spent on remove_overlapping_keys. For sparse keyspaces, we don't need to report missing key errors anyways, and it's very likely that we will need to read all layers intersecting with the key range. Therefore, this patch adds a new fast path for sparse keyspace reads that we do not track unmapped_keyspace in a fine-grained way. We only modify it when we find an image layer.

In debug mode, it was ~5min to read the aux files for a dump of the tenant, and now it's only 8s, that's a 60x speedup.

Summary of changes

• Do not add sparse keys into keys_done so that remove_overlapping does nothing.
• Allow ValueReconstructSituation::Complete to be updated again in ValuesReconstructState::update_key for sparse keyspaces.

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

[skyzh]

would like to know if @VladLazar has any opinion about, thanks :)

[github-actions]

5328 tests run: 5106 passed, 0 failed, 222 skipped (full report)

---

Flaky tests (2)

Postgres 17

• test_readonly_node_gc: debug-x86-64
• test_lsn_lease_size[False]: debug-x86-64

Code coverage* (full report)

• functions: 31.6% (7834 of 24754 functions)
• lines: 49.2% (61790 of 125491 lines)

* collected from Rust tests only

---

_{The comment gets automatically updated with the latest test results
5d7cb28 at 2024-11-06T16:22:44.580Z :recycle:}

[erikgrinaker]

Nice speedup! I'll let @VladLazar take a look as well.

This special case is worth a mention in the method comment, and any other relevant methods that call into it.

Given the performance boost, this might indicate further optimization opportunities in remove_overlapping_with() in the non-sparse case. Did you explore any other optimizations?

[VladLazar]

LGTM. I agree with Erik that we should have a comment somewhere that lists all the special handling we do for AUX keys on the read path.

[VladLazar]

Given the performance boost, this might indicate further optimization opportunities in remove_overlapping_with() in the non-sparse case. Did you explore any other optimizations?

The issue here is that the aux keyspace is huge. On non-aux queries, the keyspaces in question are much smaller (max 16 keys per vectored read), so I doubt too much time is being spent in remove_overlapping_with() there.

[skyzh]

I'll also update the RFC to include this change