Skip to content

Commit

Permalink
uploader: avoid cloning vecs just to get Bytes (#6645)
Browse files Browse the repository at this point in the history
Fix cloning the serialized heatmap on every attempt by just turning it
into `bytes::Bytes` before clone so it will be a refcounted instead of
refcounting a vec clone later on.

Also fixes one cancellation token cloning I had missed in #6618.
Cc: #6096
  • Loading branch information
koivunej authored Feb 6, 2024
1 parent 431f423 commit 5374399
Showing 1 changed file with 6 additions and 8 deletions.
14 changes: 6 additions & 8 deletions pageserver/src/tenant/secondary/heatmap_uploader.rs
Original file line number Diff line number Diff line change
Expand Up @@ -371,8 +371,6 @@ async fn upload_tenant_heatmap(
};
let timelines = tenant.timelines.lock().unwrap().clone();

let tenant_cancel = tenant.cancel.clone();

// Ensure that Tenant::shutdown waits for any upload in flight: this is needed because otherwise
// when we delete a tenant, we might race with an upload in flight and end up leaving a heatmap behind
// in remote storage.
Expand Down Expand Up @@ -401,6 +399,7 @@ async fn upload_tenant_heatmap(

// Serialize the heatmap
let bytes = serde_json::to_vec(&heatmap).map_err(|e| anyhow::anyhow!(e))?;
let bytes = bytes::Bytes::from(bytes);
let size = bytes.len();

// Drop out early if nothing changed since our last upload
Expand All @@ -411,13 +410,12 @@ async fn upload_tenant_heatmap(

let path = remote_heatmap_path(tenant.get_tenant_shard_id());

// Write the heatmap.
let cancel = &tenant.cancel;

tracing::debug!("Uploading {size} byte heatmap to {path}");
if let Err(e) = backoff::retry(
|| async {
let bytes = futures::stream::once(futures::future::ready(Ok(bytes::Bytes::from(
bytes.clone(),
))));
let bytes = futures::stream::once(futures::future::ready(Ok(bytes.clone())));
remote_storage
.upload_storage_object(bytes, size, &path)
.await
Expand All @@ -426,13 +424,13 @@ async fn upload_tenant_heatmap(
3,
u32::MAX,
"Uploading heatmap",
&tenant_cancel,
cancel,
)
.await
.ok_or_else(|| anyhow::anyhow!("Shutting down"))
.and_then(|x| x)
{
if tenant_cancel.is_cancelled() {
if cancel.is_cancelled() {
return Err(UploadHeatmapError::Cancelled);
} else {
return Err(e.into());
Expand Down

1 comment on commit 5374399

@github-actions
Copy link

Choose a reason for hiding this comment

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

2462 tests run: 2342 passed, 0 failed, 120 skipped (full report)


Flaky tests (2)

Postgres 15

  • test_crafted_wal_end[last_wal_record_crossing_segment]: debug, release

Code coverage (full report)

  • functions: 54.5% (11328 of 20789 functions)
  • lines: 81.6% (63746 of 78131 lines)

The comment gets automatically updated with the latest test results
5374399 at 2024-02-06T12:46:53.896Z :recycle:

Please sign in to comment.