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

fix(pageserver): fix gc-compaction racing with legacy gc #10052

Merged
merged 8 commits into from
Dec 9, 2024

Conversation

skyzh
Copy link
Member

@skyzh skyzh commented Dec 8, 2024

Problem

close #10049, close #10030, close #8861

part of #9114

The legacy gc process calls get_latest_gc_cutoff, which uses a Rcu different than the gc_info struct. In the gc_compaction_smoke test case, the "latest" cutoff could be lower than the gc_info struct, causing gc-compaction to collect data that could be accessed by latest_gc_cutoff. Technically speaking, there's nothing wrong with gc-compaction using gc_info without considering latest_gc_cutoff, because gc_info is the source of truth. But anyways, let's fix it.

Summary of changes

  • gc-compaction uses latest_gc_cutoff instead of gc_info to determine the gc horizon.
  • if a gc-compaction is scheduled via tenant compaction iteration, it will take the gc_block lock to avoid racing with functionalities like detach ancestor (if it's triggered via manual compaction API without scheduling, then it won't take the lock)

Copy link

github-actions bot commented Dec 8, 2024

7077 tests run: 6762 passed, 0 failed, 315 skipped (full report)


Flaky tests (5)

Postgres 17

Postgres 16

Postgres 15

Postgres 14

Code coverage* (full report)

  • functions: 31.4% (8331 of 26528 functions)
  • lines: 47.7% (65613 of 137438 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
9ddd59d at 2024-12-09T20:16:43.711Z :recycle:

@skyzh skyzh force-pushed the skyzh/try-fix-compact branch from 196c0cf to 7529510 Compare December 8, 2024 20:55
skyzh added 4 commits December 8, 2024 16:13
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 requested review from problame and arpad-m December 9, 2024 02:43
@skyzh skyzh marked this pull request as ready for review December 9, 2024 02:43
@skyzh skyzh requested a review from a team as a code owner December 9, 2024 02:43
pageserver/src/tenant.rs Outdated Show resolved Hide resolved
@skyzh skyzh enabled auto-merge December 9, 2024 18:44
@skyzh skyzh disabled auto-merge December 9, 2024 18:50
@skyzh skyzh enabled auto-merge December 9, 2024 19:52
@skyzh skyzh added this pull request to the merge queue Dec 9, 2024
Merged via the queue into main with commit 4c4cb80 Dec 9, 2024
82 checks passed
@skyzh skyzh deleted the skyzh/try-fix-compact branch December 9, 2024 20:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants