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 schedule gc-compaction #9809

Merged
merged 12 commits into from
Dec 5, 2024
Merged

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Nov 19, 2024

Problem

part of #9114

gc-compaction can take a long time. This patch adds support for scheduling a gc-compaction job. The compaction loop will first handle L0->L1 compaction, and then gc compaction. The scheduled jobs are stored in a non-persistent queue within the tenant structure.

This will be the building block for the partial compaction trigger -- if the system determines that we need to do a gc compaction, it will partition the keyspace and schedule several jobs. Each of these jobs will run for a short amount of time (i.e, 1 min). L0 compaction will be prioritized over gc compaction.

Summary of changes

  • Add compaction scheduler in tenant.
  • Run scheduled compaction in integration tests.
  • Change the manual compaction API to allow schedule a compaction instead of immediately doing it.
  • Add LSN upper bound as gc-compaction parameter. If we schedule partial compactions, gc_cutoff might move across different runs. Therefore, we need to pass a pre-determined gc_cutoff beforehand. (TODO: support LSN lower bound so that we can compact arbitrary "rectangle" in the layer map)
  • Refactor the gc_compaction internal interface.

@skyzh skyzh mentioned this pull request Nov 19, 2024
28 tasks
Copy link

github-actions bot commented Nov 19, 2024

7040 tests run: 6731 passed, 0 failed, 309 skipped (full report)


Flaky tests (2)

Postgres 16

Postgres 15

Code coverage* (full report)

  • functions: 31.4% (8324 of 26499 functions)
  • lines: 47.8% (65454 of 137023 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
7a0274c at 2024-12-05T19:49:41.936Z :recycle:

@skyzh skyzh force-pushed the skyzh/schedule-compaction branch 3 times, most recently from 1427369 to c4b2ea8 Compare November 20, 2024 21:09
@skyzh skyzh requested a review from problame November 20, 2024 21:32
@skyzh skyzh marked this pull request as ready for review November 20, 2024 21:32
@skyzh skyzh requested a review from a team as a code owner November 20, 2024 21:32
@skyzh skyzh force-pushed the skyzh/schedule-compaction branch from c343430 to ca02663 Compare November 21, 2024 17:43
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
Signed-off-by: Alex Chi Z <[email protected]>
@skyzh skyzh force-pushed the skyzh/schedule-compaction branch from 4346c23 to 676d7f6 Compare November 26, 2024 16:15
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.

It would be nice to add a management API for clearing the scheduled compactions list.
I guess it can be achieved through tenant reset API, though.


The code in fn compaction_iteration is quite messy. I think an abstraction to separate policy (what needs doing next) from the machinery to execute it (calling compact_with_options) would be appropriate.

For example:

  • comapction_iteration calls that abstraction
  • the abstraction yields aScheduledCompactionTask
  • the compaction_iteration invokes compact_with_options to execute

WDYT? Doesn't have to be done in tis PR but I'm unwilling to tolerate the messiness in the longer term.
Feel free to create a new issue that references this proposal.

pageserver/src/tenant/timeline.rs Outdated Show resolved Hide resolved
test_runner/regress/test_compaction.py Show resolved Hide resolved
@skyzh
Copy link
Member Author

skyzh commented Dec 5, 2024

the abstraction yields aScheduledCompactionTask

Yes I think we need a separation from compact options from compact task. We already have GcCompactionTask in compaction.rs. I'll do a refactor to separate them out after the patches go through.

Signed-off-by: Alex Chi Z <[email protected]>
@skyzh
Copy link
Member Author

skyzh commented Dec 5, 2024

I've created an issue to track that and I'll get it done by the next Wednesday to avoid having tech debt in the code #10031

@skyzh skyzh enabled auto-merge December 5, 2024 18:26
Signed-off-by: Alex Chi Z <[email protected]>
@skyzh skyzh added this pull request to the merge queue Dec 5, 2024
Merged via the queue into main with commit 71f38d1 Dec 5, 2024
78 checks passed
@skyzh skyzh deleted the skyzh/schedule-compaction branch December 5, 2024 19:39
github-merge-queue bot pushed a commit that referenced this pull request Dec 6, 2024
## Problem

part of #9114, stacked PR
over #9809

The compaction scheduler now schedules partial compaction jobs.

## Summary of changes

* Add the compaction job splitter based on size.
* Schedule subcompactions using the compaction scheduler.
* Test subcompaction scheduler in the smoke regress test.
* Temporarily disable layer map checks

---------

Signed-off-by: Alex Chi Z <[email protected]>
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