Skip to content

Commit

Permalink
refactor(compaction): RequestContext shouldn't be Clone, only `Re…
Browse files Browse the repository at this point in the history
…questContextAdaptor` uses it (#6961)

Extracted from #6953

Part of #5899
  • Loading branch information
problame committed Feb 29, 2024
1 parent 76ab57f commit 502b69b
Showing 1 changed file with 10 additions and 24 deletions.
34 changes: 10 additions & 24 deletions pageserver/src/tenant/timeline/compaction.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,13 @@ impl Timeline {

let keyspace = self.collect_keyspace(end_lsn, ctx).await?;
let mut adaptor = TimelineAdaptor::new(self, (end_lsn, keyspace));
let ctx_adaptor = RequestContextAdaptor(ctx.clone());

pageserver_compaction::compact_tiered::compact_tiered(
&mut adaptor,
end_lsn,
target_file_size,
fanout,
&ctx_adaptor,
ctx,
)
.await?;

Expand Down Expand Up @@ -143,13 +142,13 @@ impl CompactionJobExecutor for TimelineAdaptor {
type DeltaLayer = ResidentDeltaLayer;
type ImageLayer = ResidentImageLayer;

type RequestContext = RequestContextAdaptor;
type RequestContext = crate::context::RequestContext;

async fn get_layers(
&mut self,
key_range: &Range<Key>,
lsn_range: &Range<Lsn>,
_ctx: &RequestContextAdaptor,
_ctx: &RequestContext,
) -> anyhow::Result<Vec<OwnArc<PersistentLayerDesc>>> {
self.flush_updates().await?;

Expand All @@ -170,7 +169,7 @@ impl CompactionJobExecutor for TimelineAdaptor {
&mut self,
key_range: &Range<Key>,
lsn: Lsn,
_ctx: &RequestContextAdaptor,
_ctx: &RequestContext,
) -> anyhow::Result<Vec<Range<Key>>> {
if lsn == self.keyspace.0 {
Ok(pageserver_compaction::helpers::intersect_keyspace(
Expand Down Expand Up @@ -206,7 +205,7 @@ impl CompactionJobExecutor for TimelineAdaptor {
&mut self,
lsn: Lsn,
key_range: &Range<Key>,
ctx: &RequestContextAdaptor,
ctx: &RequestContext,
) -> anyhow::Result<()> {
Ok(self.create_image_impl(lsn, key_range, ctx).await?)
}
Expand All @@ -216,7 +215,7 @@ impl CompactionJobExecutor for TimelineAdaptor {
lsn_range: &Range<Lsn>,
key_range: &Range<Key>,
input_layers: &[ResidentDeltaLayer],
ctx: &RequestContextAdaptor,
ctx: &RequestContext,
) -> anyhow::Result<()> {
debug!("Create new layer {}..{}", lsn_range.start, lsn_range.end);

Expand Down Expand Up @@ -287,7 +286,7 @@ impl CompactionJobExecutor for TimelineAdaptor {
async fn delete_layer(
&mut self,
layer: &OwnArc<PersistentLayerDesc>,
_ctx: &RequestContextAdaptor,
_ctx: &RequestContext,
) -> anyhow::Result<()> {
self.layers_to_delete.push(layer.clone().0);
Ok(())
Expand All @@ -299,7 +298,7 @@ impl TimelineAdaptor {
&mut self,
lsn: Lsn,
key_range: &Range<Key>,
ctx: &RequestContextAdaptor,
ctx: &RequestContext,
) -> Result<(), PageReconstructError> {
let timer = self.timeline.metrics.create_images_time_histo.start_timer();

Expand Down Expand Up @@ -361,17 +360,7 @@ impl TimelineAdaptor {
}
}

pub struct RequestContextAdaptor(pub RequestContext);

impl std::ops::Deref for RequestContextAdaptor {
type Target = RequestContext;

fn deref(&self) -> &Self::Target {
&self.0
}
}

impl CompactionRequestContext for RequestContextAdaptor {}
impl CompactionRequestContext for crate::context::RequestContext {}

#[derive(Debug, Clone)]
pub struct OwnArc<T>(pub Arc<T>);
Expand Down Expand Up @@ -449,10 +438,7 @@ impl CompactionLayer<Key> for ResidentDeltaLayer {
impl CompactionDeltaLayer<TimelineAdaptor> for ResidentDeltaLayer {
type DeltaEntry<'a> = DeltaEntry<'a>;

async fn load_keys<'a>(
&self,
ctx: &RequestContextAdaptor,
) -> anyhow::Result<Vec<DeltaEntry<'_>>> {
async fn load_keys<'a>(&self, ctx: &RequestContext) -> anyhow::Result<Vec<DeltaEntry<'_>>> {
self.0.load_keys(ctx).await
}
}
Expand Down

1 comment on commit 502b69b

@github-actions
Copy link

Choose a reason for hiding this comment

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

2525 tests run: 2389 passed, 0 failed, 136 skipped (full report)


Code coverage* (full report)

  • functions: 28.6% (6887 of 24064 functions)
  • lines: 47.1% (42121 of 89457 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
502b69b at 2024-02-29T20:46:56.408Z :recycle:

Please sign in to comment.