Skip to content

Commit

Permalink
pageserver: use Gate for stronger safety check in SlotGuard (#5793)
Browse files Browse the repository at this point in the history
## Problem

#5711 and #5367 raced -- the `SlotGuard` type needs `Gate` to properly
enforce its invariant that we may not drop an `Arc<Tenant>` from a slot.

## Summary of changes

Replace the TODO with the intended check of Gate.
  • Loading branch information
jcsp authored Nov 8, 2023
1 parent a8a39cd commit 40441f8
Show file tree
Hide file tree
Showing 2 changed files with 8 additions and 8 deletions.
7 changes: 7 additions & 0 deletions libs/utils/src/sync/gate.rs
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,13 @@ impl Gate {
warn_if_stuck(self.do_close(), &self.name, Duration::from_millis(1000)).await
}

/// Check if [`Self::close()`] has finished waiting for all [`Self::enter()`] users to finish. This
/// is usually analoguous for "Did shutdown finish?" for types that include a Gate, whereas checking
/// the CancellationToken on such types is analogous to "Did shutdown start?"
pub fn close_complete(&self) -> bool {
self.sem.is_closed()
}

async fn do_close(&self) {
tracing::debug!(gate = self.name, "Closing Gate...");
match self.sem.acquire_many(Self::MAX_UNITS).await {
Expand Down
9 changes: 1 addition & 8 deletions pageserver/src/tenant/mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1552,14 +1552,7 @@ impl SlotGuard {
/// is responsible for protecting
fn old_value_is_shutdown(&self) -> bool {
match self.old_value.as_ref() {
Some(TenantSlot::Attached(tenant)) => {
// TODO: PR #5711 will add a gate that enables properly checking that
// shutdown completed.
matches!(
tenant.current_state(),
TenantState::Stopping { .. } | TenantState::Broken { .. }
)
}
Some(TenantSlot::Attached(tenant)) => tenant.gate.close_complete(),
Some(TenantSlot::Secondary) => {
// TODO: when adding secondary mode tenants, this will check for shutdown
// in the same way that we do for `Tenant` above
Expand Down

1 comment on commit 40441f8

@github-actions
Copy link

Choose a reason for hiding this comment

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

2438 tests run: 2317 passed, 0 failed, 121 skipped (full report)


Flaky tests (2)

Postgres 16

  • test_restarts_frequent_checkpoints: release

Postgres 15

  • test_branching_with_pgbench[cascade-1-10]: debug

Code coverage (full report)

  • functions: 54.7% (8898 of 16258 functions)
  • lines: 81.7% (51265 of 62763 lines)

The comment gets automatically updated with the latest test results
40441f8 at 2023-11-08T13:42:40.862Z :recycle:

Please sign in to comment.