Skip to content

Commit

Permalink
pageserver: use 200 instead of 404 on deletion success
Browse files Browse the repository at this point in the history
  • Loading branch information
jcsp committed Jun 17, 2024
1 parent 23f3f29 commit 468941b
Show file tree
Hide file tree
Showing 2 changed files with 5 additions and 13 deletions.
11 changes: 2 additions & 9 deletions pageserver/src/http/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,7 @@ async fn tenant_delete_handler(

let state = get_state(&request);

let status = state
state
.tenant_manager
.delete_tenant(tenant_shard_id)
.instrument(info_span!("tenant_delete_handler",
Expand All @@ -1077,14 +1077,7 @@ async fn tenant_delete_handler(
))
.await?;

// Callers use 404 as success for deletions, for historical reasons.
if status == StatusCode::NOT_FOUND {
return Err(ApiError::NotFound(
anyhow::anyhow!("Deletion complete").into(),
));
}

json_response(status, ())
json_response(StatusCode::OK, ())
}

/// HTTP endpoint to query the current tenant_size of a tenant.
Expand Down
7 changes: 3 additions & 4 deletions pageserver/src/tenant/mgr.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@

use camino::{Utf8DirEntry, Utf8Path, Utf8PathBuf};
use futures::StreamExt;
use hyper::StatusCode;
use itertools::Itertools;
use pageserver_api::key::Key;
use pageserver_api::models::LocationConfigMode;
Expand Down Expand Up @@ -1374,7 +1373,7 @@ impl TenantManager {
pub(crate) async fn delete_tenant(
&self,
tenant_shard_id: TenantShardId,
) -> Result<StatusCode, DeleteTenantError> {
) -> Result<(), DeleteTenantError> {
super::span::debug_assert_current_span_has_tenant_id();

async fn delete_local(
Expand Down Expand Up @@ -1433,7 +1432,7 @@ impl TenantManager {
Err(remote_storage::DownloadError::Cancelled) => {
return Err(DeleteTenantError::Cancelled)
}
Err(remote_storage::DownloadError::NotFound) => return Ok(StatusCode::NOT_FOUND),
Err(remote_storage::DownloadError::NotFound) => return Ok(()),
Err(other) => return Err(DeleteTenantError::Other(anyhow::anyhow!(other))),
};

Expand All @@ -1448,7 +1447,7 @@ impl TenantManager {
}

// Callers use 404 as success for deletions, for historical reasons.
Ok(StatusCode::NOT_FOUND)
Ok(())
}

#[instrument(skip_all, fields(tenant_id=%tenant.get_tenant_shard_id().tenant_id, shard_id=%tenant.get_tenant_shard_id().shard_slug(), new_shard_count=%new_shard_count.literal()))]
Expand Down

0 comments on commit 468941b

Please sign in to comment.