[Bug]: Cascading priority deadlock in Pageserver read path via `gc_compaction_layer_update_lock` during layer I/O
Description
There is a critical liveness vulnerability in the Pageserver's read path, specifically within Timeline::get_vectored_reconstruct_data_timeline() (pageserver/src/tenant/timeline.rs).
The code holds a tokio::sync::RwLockReadGuard on gc_compaction_layer_update_lock across multiple unbounded .await points (layer I/O operations including disk reads and potential remote storage downloads). Because Tokio's RwLock enforces a strict writer-priority (fair) queue to prevent writer starvation, holding a read lock during slow layer I/O creates a trap: when compact_with_gc_inner() requests a write lock (to atomically update the layer map after GC compaction), the writer will block waiting for the slow reader, and all subsequent page read requests will be permanently queued behind the pending writer.
This completely stalls the page read pipeline until the original slow I/O completes, leading to query timeouts and compute node disconnections under specific timing conditions.
Code Snippet (read path — get_vectored_reconstruct_data_timeline):
// pageserver/src/tenant/timeline.rs:4670-4756
async fn get_vectored_reconstruct_data_timeline(
timeline: &Timeline,
query: &VersionedKeySpaceQuery,
reconstruct_state: &mut ValuesReconstructState,
cancel: &CancellationToken,
ctx: &RequestContext,
) -> Result<TimelineVisitOutcome, GetVectoredError> {
let _gc_cutoff_holder = timeline.get_applied_gc_cutoff_lsn();
// Line 4685: DANGER: Read guard acquired here
let _guard = timeline.gc_compaction_layer_update_lock.read().await;
// Line 4688: Layer traversal begins — involves unbounded I/O
let mut fringe = timeline.get_vectored_init_fringe(query).await?;
while let Some((layer_to_read, keyspace_to_read, lsn_range)) = fringe.next_layer() {
// Line 4704-4711: DANGER: Awaiting disk/network I/O while holding ReadGuard
layer_to_read
.get_values_reconstruct_data(
keyspace_to_read.clone(),
lsn_range,
reconstruct_state,
ctx,
)
.await?;
// Line 4735: Additional lock acquisition within the same guard scope
if !unmapped_keyspace.is_empty() {
let guard = timeline.layers.read(LayerManagerLockHolder::GetPage).await;
guard.update_search_fringe(&unmapped_keyspace, cont_lsn, &mut fringe)?;
}
}
// _guard dropped here — read lock held for ENTIRE layer traversal
}Code Snippet (write path — compact_with_gc_inner):
// pageserver/src/tenant/timeline/compaction.rs:4143
// This requests a WRITE lock on the same RwLock
let update_guard = self.gc_compaction_layer_update_lock.write().await;
// Acquiring the update guard ensures current read operations end
// and new read operations are blocked.
let mut guard = self.layers.write(LayerManagerLockHolder::GarbageCollection).await;
guard.open_mut()?.finish_gc_compaction(&layer_selection, &compact_to, &self.metrics);
drop(update_guard); // Allow new reads to start ONLY after layer map updatedSteps to reproduce
Minimal Reproducible Example (Rust):
This MRE demonstrates the exact locking pattern used in the Pageserver — a reader holds RwLock::read() across a long I/O simulation, a writer (gc-compaction) requests RwLock::write(), and subsequent readers (new page requests) are all blocked.
use std::sync::Arc;
use tokio::sync::RwLock;
use tokio::time::{sleep, Duration, Instant};
/// Simulates the Pageserver's gc_compaction_layer_update_lock pattern:
/// - Task A: get_vectored_reconstruct_data_timeline (reader holding lock across I/O)
/// - Task B: compact_with_gc_inner (writer requesting lock for layer map update)
/// - Task C: New page read request (reader blocked by writer-priority policy)
#[tokio::main]
async fn main() {
let lock = Arc::new(RwLock::new(()));
// Task A: Simulating get_vectored_reconstruct_data_timeline
// Acquires read lock and holds it across layer I/O (e.g., reading from remote storage)
let lock_a = lock.clone();
tokio::spawn(async move {
println!("[get_vectored] Requesting gc_compaction_layer_update_lock.read()...");
let _guard = lock_a.read().await;
println!("[get_vectored] Read lock acquired. Traversing layers with I/O (3s)...");
// Simulates: layer_to_read.get_values_reconstruct_data().await
// In production, this could be slow if layer needs download from S3
sleep(Duration::from_secs(3)).await;
println!("[get_vectored] Layer traversal complete. Releasing read lock.");
});
// Ensure Task A gets the lock first
sleep(Duration::from_millis(100)).await;
// Task B: Simulating compact_with_gc_inner requesting write lock
let lock_b = lock.clone();
tokio::spawn(async move {
println!("[gc_compaction] Requesting gc_compaction_layer_update_lock.write()...");
let _guard = lock_b.write().await;
println!("[gc_compaction] Write lock acquired. Updating layer map.");
});
// Ensure Task B is in the queue
sleep(Duration::from_millis(100)).await;
// Task C: New page read request — simulates a compute query
let lock_c = lock.clone();
let start_time = Instant::now();
tokio::spawn(async move {
println!("[new_page_read] Compute requests a page! Requesting read lock...");
let _guard = lock_c.read().await; // Blocks here unexpectedly!
println!(
"[new_page_read] Read lock acquired after {:?} — query was stalled!",
start_time.elapsed()
);
})
.await
.unwrap();
}Expected Result vs Actual Execution:
Naive Expectation: Since Task A holds a read lock, one might expect Task C (another reader) to instantly acquire the read lock and execute concurrently with Task A, while Task B (the writer) waits for both readers to finish.
Actual Execution:
[get_vectored] Requesting gc_compaction_layer_update_lock.read()...
[get_vectored] Read lock acquired. Traversing layers with I/O (3s)...
[gc_compaction] Requesting gc_compaction_layer_update_lock.write()...
[new_page_read] Compute requests a page! Requesting read lock...
[get_vectored] Layer traversal complete. Releasing read lock.
[gc_compaction] Write lock acquired. Updating layer map.
[new_page_read] Read lock acquired after 2.9xxs — query was stalled!
Explanation: Task C (new page read) is completely blocked by Task B's pending write request, even though Task C only needs a read lock. In the context of the Pageserver:
- A page read (
get_vectored) is executing and holdsgc_compaction_layer_update_lock.read()across multiple layer I/O operations - GC compaction finishes its work and needs
gc_compaction_layer_update_lock.write()to atomically update the layer map — it queues behind the reader - All subsequent page read requests are blocked by tokio's write-preferring policy, queuing behind the pending writer
- If the original reader's I/O is slow (remote storage latency, S3 timeout), hundreds of page reads can be stalled for seconds
This is confirmed by tokio 1.49.0 documentation:
"The priority policy of Tokio's read-write lock is fair (or write-preferring)... if a task that wishes to acquire the write lock is at the head of the queue, read locks will not be given out until the write lock has been released."
Expected result
Page read requests should not be significantly delayed by GC compaction's need to update the layer map. The gc_compaction_layer_update_lock read guard should be held for the minimum necessary duration, not across the entire layer traversal.
Actual result
Under concurrent read load + GC compaction, all new page read requests are blocked when:
- Any existing reader's layer I/O is slow (common with remote storage)
- GC compaction requests the write lock
This creates a cascading stall that can last for the duration of the slowest reader's I/O operation + the write operation time.
Environment
- Neon Pageserver with
gc_compaction_enabled = true tokio::sync::RwLockwrite-preferring (fair) policy (tokio >= 1.x)- Timeline with many layers requiring remote storage access
- Concurrent page read load from compute nodes
Possible Fix
Consider one of:
- Narrow the read guard scope: Release
gc_compaction_layer_update_lock.read()between layer visits, re-acquiring only when needed - Use
std::sync::RwLockif the critical section doesn't need to span across.awaitpoints (which would require restructuring) - Use an RCU-like approach: The code comment at line 4145 already suggests this: "TODO: can we use
latest_gc_cutoffRcu to achieve the same effect?" - Use
tokio::sync::Semaphorewith a large permit count for readers and drain pattern for writers, avoiding the write-preferring starvation
Source: neondatabase/neon