Skip to content

Commit

Permalink
pageserver: fix return code from secondary_download_handler (neondata…
Browse files Browse the repository at this point in the history
…base#8508)

## Problem

The secondary download HTTP API is meant to return 200 if the download
is complete, and 202 if it is still in progress. In neondatabase#8198 the download
implementation was changed to drop out with success early if it
over-runs a time budget, which resulted in 200 responses for incomplete
downloads.

This breaks storcon_cli's "tenant-warmup" command, which uses the OK
status to indicate download complete.

## Summary of changes

- Only return 200 if we get an Ok() _and_ the progress stats indicate
the download is complete.
  • Loading branch information
jcsp authored Jul 29, 2024
1 parent bdfc9ca commit 5775662
Showing 1 changed file with 15 additions and 7 deletions.
22 changes: 15 additions & 7 deletions pageserver/src/http/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2129,14 +2129,24 @@ async fn secondary_download_handler(

let timeout = wait.unwrap_or(Duration::MAX);

let status = match tokio::time::timeout(
let result = tokio::time::timeout(
timeout,
state.secondary_controller.download_tenant(tenant_shard_id),
)
.await
{
// Download job ran to completion.
Ok(Ok(())) => StatusCode::OK,
.await;

let progress = secondary_tenant.progress.lock().unwrap().clone();

let status = match result {
Ok(Ok(())) => {
if progress.layers_downloaded >= progress.layers_total {
// Download job ran to completion
StatusCode::OK
} else {
// Download dropped out without errors because it ran out of time budget
StatusCode::ACCEPTED
}
}
// Edge case: downloads aren't usually fallible: things like a missing heatmap are considered
// okay. We could get an error here in the unlikely edge case that the tenant
// was detached between our check above and executing the download job.
Expand All @@ -2146,8 +2156,6 @@ async fn secondary_download_handler(
Err(_) => StatusCode::ACCEPTED,
};

let progress = secondary_tenant.progress.lock().unwrap().clone();

json_response(status, progress)
}

Expand Down

0 comments on commit 5775662

Please sign in to comment.