diff --git a/pageserver/src/context.rs b/pageserver/src/context.rs index da9c095a15..072f8081c3 100644 --- a/pageserver/src/context.rs +++ b/pageserver/src/context.rs @@ -92,7 +92,7 @@ use crate::task_mgr::TaskKind; // The main structure of this module, see module-level comment. -#[derive(Debug)] +#[derive(Clone)] pub struct RequestContext { task_kind: TaskKind, download_behavior: DownloadBehavior, @@ -161,20 +161,17 @@ impl RequestContextBuilder { } } - pub fn extend(original: &RequestContext) -> Self { + pub fn from(original: &RequestContext) -> Self { Self { - // This is like a Copy, but avoid implementing Copy because ordinary users of - // RequestContext should always move or ref it. - inner: RequestContext { - task_kind: original.task_kind, - download_behavior: original.download_behavior, - access_stats_behavior: original.access_stats_behavior, - page_content_kind: original.page_content_kind, - read_path_debug: original.read_path_debug, - }, + inner: original.clone(), } } + pub fn task_kind(mut self, b: TaskKind) -> Self { + self.inner.task_kind = b; + self + } + /// Configure the DownloadBehavior of the context: whether to /// download missing layers, and/or warn on the download. pub fn download_behavior(mut self, b: DownloadBehavior) -> Self { @@ -199,7 +196,15 @@ impl RequestContextBuilder { self } - pub fn build(self) -> RequestContext { + pub fn root(self) -> RequestContext { + self.inner + } + + pub fn attached_child(self) -> RequestContext { + self.inner + } + + pub fn detached_child(self) -> RequestContext { self.inner } } @@ -220,7 +225,7 @@ impl RequestContext { pub fn new(task_kind: TaskKind, download_behavior: DownloadBehavior) -> Self { RequestContextBuilder::new(task_kind) .download_behavior(download_behavior) - .build() + .root() } /// Create a detached child context for a task that may outlive `self`. @@ -241,7 +246,10 @@ impl RequestContext { /// /// We could make new calls to this function fail if `self` is already canceled. pub fn detached_child(&self, task_kind: TaskKind, download_behavior: DownloadBehavior) -> Self { - self.child_impl(task_kind, download_behavior) + RequestContextBuilder::from(self) + .task_kind(task_kind) + .download_behavior(download_behavior) + .detached_child() } /// Create a child of context `self` for a task that shall not outlive `self`. @@ -265,7 +273,7 @@ impl RequestContext { /// The method to wait for child tasks would return an error, indicating /// that the child task was not started because the context was canceled. pub fn attached_child(&self) -> Self { - self.child_impl(self.task_kind(), self.download_behavior()) + RequestContextBuilder::from(self).attached_child() } /// Use this function when you should be creating a child context using @@ -280,10 +288,6 @@ impl RequestContext { Self::new(task_kind, download_behavior) } - fn child_impl(&self, task_kind: TaskKind, download_behavior: DownloadBehavior) -> Self { - Self::new(task_kind, download_behavior) - } - pub fn task_kind(&self) -> TaskKind { self.task_kind } diff --git a/pageserver/src/http/routes.rs b/pageserver/src/http/routes.rs index cd79aa6680..26015adcf9 100644 --- a/pageserver/src/http/routes.rs +++ b/pageserver/src/http/routes.rs @@ -2579,9 +2579,10 @@ async fn getpage_at_lsn_handler_inner( let lsn: Option = parse_query_param(&request, "lsn")?; async { - let ctx = RequestContext::new(TaskKind::MgmtRequest, DownloadBehavior::Download); - // Enable read path debugging - let ctx = RequestContextBuilder::extend(&ctx).read_path_debug(true).build(); + let ctx = RequestContextBuilder::new(TaskKind::MgmtRequest) + .download_behavior(DownloadBehavior::Download) + .read_path_debug(true) + .root(); let timeline = active_timeline_of_active_tenant(&state.tenant_manager, tenant_shard_id, timeline_id).await?; // Use last_record_lsn if no lsn is provided diff --git a/pageserver/src/tenant/storage_layer/delta_layer.rs b/pageserver/src/tenant/storage_layer/delta_layer.rs index 83ac6aab51..300d0ae00e 100644 --- a/pageserver/src/tenant/storage_layer/delta_layer.rs +++ b/pageserver/src/tenant/storage_layer/delta_layer.rs @@ -896,9 +896,9 @@ impl DeltaLayerInner { where Reader: BlockReader + Clone, { - let ctx = RequestContextBuilder::extend(ctx) + let ctx = RequestContextBuilder::from(ctx) .page_content_kind(PageContentKind::DeltaLayerBtreeNode) - .build(); + .attached_child(); for range in keyspace.ranges.iter() { let mut range_end_handled = false; @@ -1105,9 +1105,9 @@ impl DeltaLayerInner { all_keys.push(entry); true }, - &RequestContextBuilder::extend(ctx) + &RequestContextBuilder::from(ctx) .page_content_kind(PageContentKind::DeltaLayerBtreeNode) - .build(), + .attached_child(), ) .await?; if let Some(last) = all_keys.last_mut() { diff --git a/pageserver/src/tenant/storage_layer/image_layer.rs b/pageserver/src/tenant/storage_layer/image_layer.rs index 0db9e8c845..afa8a20c31 100644 --- a/pageserver/src/tenant/storage_layer/image_layer.rs +++ b/pageserver/src/tenant/storage_layer/image_layer.rs @@ -481,9 +481,9 @@ impl ImageLayerInner { let tree_reader = DiskBtreeReader::new(self.index_start_blk, self.index_root_blk, block_reader); - let ctx = RequestContextBuilder::extend(ctx) + let ctx = RequestContextBuilder::from(ctx) .page_content_kind(PageContentKind::ImageLayerBtreeNode) - .build(); + .attached_child(); for range in keyspace.ranges.iter() { let mut range_end_handled = false; diff --git a/pageserver/src/tenant/storage_layer/inmemory_layer.rs b/pageserver/src/tenant/storage_layer/inmemory_layer.rs index 46135b5330..42b581ee3c 100644 --- a/pageserver/src/tenant/storage_layer/inmemory_layer.rs +++ b/pageserver/src/tenant/storage_layer/inmemory_layer.rs @@ -420,9 +420,9 @@ impl InMemoryLayer { reconstruct_state: &mut ValuesReconstructState, ctx: &RequestContext, ) -> Result<(), GetVectoredError> { - let ctx = RequestContextBuilder::extend(ctx) + let ctx = RequestContextBuilder::from(ctx) .page_content_kind(PageContentKind::InMemoryLayer) - .build(); + .attached_child(); let inner = self.inner.read().await; diff --git a/pageserver/src/tenant/storage_layer/layer.rs b/pageserver/src/tenant/storage_layer/layer.rs index bde7fbc1f9..a8d3d27caf 100644 --- a/pageserver/src/tenant/storage_layer/layer.rs +++ b/pageserver/src/tenant/storage_layer/layer.rs @@ -1720,9 +1720,9 @@ impl DownloadedLayer { ); let res = if owner.desc.is_delta { - let ctx = RequestContextBuilder::extend(ctx) + let ctx = RequestContextBuilder::from(ctx) .page_content_kind(crate::context::PageContentKind::DeltaLayerSummary) - .build(); + .attached_child(); let summary = Some(delta_layer::Summary::expected( owner.desc.tenant_shard_id.tenant_id, owner.desc.timeline_id, @@ -1738,9 +1738,9 @@ impl DownloadedLayer { .await .map(LayerKind::Delta) } else { - let ctx = RequestContextBuilder::extend(ctx) + let ctx = RequestContextBuilder::from(ctx) .page_content_kind(crate::context::PageContentKind::ImageLayerSummary) - .build(); + .attached_child(); let lsn = owner.desc.image_layer_lsn(); let summary = Some(image_layer::Summary::expected( owner.desc.tenant_shard_id.tenant_id, diff --git a/pageserver/src/tenant/storage_layer/layer/tests.rs b/pageserver/src/tenant/storage_layer/layer/tests.rs index a7f3c6b8c5..dbf49a8ce6 100644 --- a/pageserver/src/tenant/storage_layer/layer/tests.rs +++ b/pageserver/src/tenant/storage_layer/layer/tests.rs @@ -644,9 +644,9 @@ async fn cancelled_get_or_maybe_download_does_not_cancel_eviction() { .unwrap(); // This test does downloads - let ctx = RequestContextBuilder::extend(&ctx) + let ctx = RequestContextBuilder::from(&ctx) .download_behavior(DownloadBehavior::Download) - .build(); + .attached_child(); let layer = { let mut layers = { @@ -729,9 +729,9 @@ async fn evict_and_wait_does_not_wait_for_download() { .unwrap(); // This test does downloads - let ctx = RequestContextBuilder::extend(&ctx) + let ctx = RequestContextBuilder::from(&ctx) .download_behavior(DownloadBehavior::Download) - .build(); + .attached_child(); let layer = { let mut layers = { diff --git a/pageserver/src/tenant/timeline.rs b/pageserver/src/tenant/timeline.rs index f646e621d3..3d4aaea466 100644 --- a/pageserver/src/tenant/timeline.rs +++ b/pageserver/src/tenant/timeline.rs @@ -7184,9 +7184,9 @@ mod tests { eprintln!("Downloading {layer} and re-generating heatmap"); - let ctx = &RequestContextBuilder::extend(&ctx) + let ctx = &RequestContextBuilder::from(&ctx) .download_behavior(crate::context::DownloadBehavior::Download) - .build(); + .attached_child(); let _resident = layer .download_and_keep_resident(ctx) diff --git a/pageserver/src/tenant/timeline/compaction.rs b/pageserver/src/tenant/timeline/compaction.rs index 17f7d96e5e..ccad9e5543 100644 --- a/pageserver/src/tenant/timeline/compaction.rs +++ b/pageserver/src/tenant/timeline/compaction.rs @@ -928,9 +928,9 @@ impl Timeline { { Ok(((dense_partitioning, sparse_partitioning), lsn)) => { // Disables access_stats updates, so that the files we read remain candidates for eviction after we're done with them - let image_ctx = RequestContextBuilder::extend(ctx) + let image_ctx = RequestContextBuilder::from(ctx) .access_stats_behavior(AccessStatsBehavior::Skip) - .build(); + .attached_child(); let mut partitioning = dense_partitioning; partitioning