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

feat(pageserver): support key range for manual compaction trigger #9723

Open
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Nov 11, 2024

Problem

part of #9114, we want to be able to run partial gc-compaction in tests. In the future, we can also expand this functionality to legacy compaction, so that we can trigger compaction for a specific key range.

Summary of changes

  • Support passing compaction key range through pageserver routes.
  • Refactor input parameters of compact related function to take the new CompactOptions.
  • Add tests for partial compaction. Note that the test may or may not trigger compaction based on GC horizon. We need to improve the test case to ensure things always get below the gc_horizon and the gc-compaction can be triggered.

@skyzh skyzh mentioned this pull request Nov 11, 2024
18 tasks
Copy link

github-actions bot commented Nov 11, 2024

5427 tests run: 5184 passed, 0 failed, 243 skipped (full report)


Flaky tests (1)

Postgres 16

Code coverage* (full report)

  • functions: 31.8% (7895 of 24863 functions)
  • lines: 49.4% (62499 of 126456 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
71b20cf at 2024-11-14T21:31:53.291Z :recycle:

@skyzh skyzh force-pushed the skyzh/partial-compaction-manual branch 3 times, most recently from 6aa7515 to 27e70aa Compare November 14, 2024 18:14
@skyzh skyzh requested a review from problame November 14, 2024 18:14
@skyzh skyzh marked this pull request as ready for review November 14, 2024 18:14
@skyzh skyzh requested a review from a team as a code owner November 14, 2024 18:14
@skyzh skyzh requested a review from arpad-m November 14, 2024 18:58
@skyzh skyzh force-pushed the skyzh/partial-compaction-manual branch from 27e70aa to 59c42fa Compare November 14, 2024 20:06
Signed-off-by: Alex Chi Z <[email protected]>
Copy link
Contributor

@problame problame left a comment

Choose a reason for hiding this comment

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

Occupying the entire request body for just the CompactRange will make it harder to extend the endpoint down the line.

I think it shouldn't be particularly hard/time consuming to move all the query fields (force_l0_compaction, force_repartition etc) into a struct TimelineCompactRequest.

(The reason why it shouldn't be particularly time consuming is that the the only place where we're constructing the request is in the ps_http.timeline_compact() implementation.

Please do bit that bullet here, much appreciated!

Other than that, this looks good. Approving so you can merge this before your weekend starts

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.

2 participants