Skip to content

Commit

Permalink
pageserver: remove code that resumes tenant deletions after restarts (#…
Browse files Browse the repository at this point in the history
…8091)

#8082 removed the legacy deletion path, but retained code for completing
deletions that were started before a pageserver restart. This PR cleans
up that remaining code, and removes all the pageserver code that dealt
with tenant deletion markers and resuming tenant deletions.

The release at #8138 contains
#8082, so we can now merge this
to `main`
  • Loading branch information
jcsp committed Jun 24, 2024
1 parent 5446e08 commit 188797f
Show file tree
Hide file tree
Showing 6 changed files with 22 additions and 553 deletions.
12 changes: 1 addition & 11 deletions pageserver/src/config.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,7 @@ use utils::{
use crate::tenant::timeline::GetVectoredImpl;
use crate::tenant::vectored_blob_io::MaxVectoredReadBytes;
use crate::tenant::{config::TenantConfOpt, timeline::GetImpl};
use crate::tenant::{
TENANTS_SEGMENT_NAME, TENANT_DELETED_MARKER_FILE_NAME, TIMELINES_SEGMENT_NAME,
};
use crate::tenant::{TENANTS_SEGMENT_NAME, TIMELINES_SEGMENT_NAME};
use crate::{disk_usage_eviction_task::DiskUsageEvictionTaskConfig, virtual_file::io_engine};
use crate::{tenant::config::TenantConf, virtual_file};
use crate::{
Expand Down Expand Up @@ -855,14 +853,6 @@ impl PageServerConf {
)
}

pub(crate) fn tenant_deleted_mark_file_path(
&self,
tenant_shard_id: &TenantShardId,
) -> Utf8PathBuf {
self.tenant_path(tenant_shard_id)
.join(TENANT_DELETED_MARKER_FILE_NAME)
}

pub fn traces_path(&self) -> Utf8PathBuf {
self.workdir.join("traces")
}
Expand Down
9 changes: 3 additions & 6 deletions pageserver/src/http/routes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -329,14 +329,11 @@ impl From<crate::tenant::mgr::DeleteTimelineError> for ApiError {
}
}

impl From<crate::tenant::delete::DeleteTenantError> for ApiError {
fn from(value: crate::tenant::delete::DeleteTenantError) -> Self {
use crate::tenant::delete::DeleteTenantError::*;
impl From<crate::tenant::mgr::DeleteTenantError> for ApiError {
fn from(value: crate::tenant::mgr::DeleteTenantError) -> Self {
use crate::tenant::mgr::DeleteTenantError::*;
match value {
Get(g) => ApiError::from(g),
Timeline(t) => ApiError::from(t),
SlotError(e) => e.into(),
SlotUpsertError(e) => e.into(),
Other(o) => ApiError::InternalServerError(o),
Cancelled => ApiError::ShuttingDown,
}
Expand Down
69 changes: 2 additions & 67 deletions pageserver/src/tenant.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,11 +55,9 @@ use self::config::AttachedLocationConfig;
use self::config::AttachmentMode;
use self::config::LocationConf;
use self::config::TenantConf;
use self::delete::DeleteTenantFlow;
use self::metadata::TimelineMetadata;
use self::mgr::GetActiveTenantError;
use self::mgr::GetTenantError;
use self::mgr::TenantsMap;
use self::remote_timeline_client::upload::upload_index_part;
use self::remote_timeline_client::RemoteTimelineClient;
use self::timeline::uninit::TimelineCreateGuard;
Expand Down Expand Up @@ -137,7 +135,6 @@ pub mod remote_timeline_client;
pub mod storage_layer;

pub mod config;
pub mod delete;
pub mod mgr;
pub mod secondary;
pub mod tasks;
Expand All @@ -161,8 +158,6 @@ pub const TENANTS_SEGMENT_NAME: &str = "tenants";
/// Parts of the `.neon/tenants/<tenant_id>/timelines/<timeline_id>` directory prefix.
pub const TIMELINES_SEGMENT_NAME: &str = "timelines";

pub const TENANT_DELETED_MARKER_FILE_NAME: &str = "deleted";

/// References to shared objects that are passed into each tenant, such
/// as the shared remote storage client and process initialization state.
#[derive(Clone)]
Expand Down Expand Up @@ -207,7 +202,6 @@ struct TimelinePreload {
}

pub(crate) struct TenantPreload {
deleting: bool,
timelines: HashMap<TimelineId, TimelinePreload>,
}

Expand Down Expand Up @@ -286,8 +280,6 @@ pub struct Tenant {
/// background warmup.
pub(crate) activate_now_sem: tokio::sync::Semaphore,

pub(crate) delete_progress: Arc<tokio::sync::Mutex<DeleteTenantFlow>>,

// Cancellation token fires when we have entered shutdown(). This is a parent of
// Timelines' cancellation token.
pub(crate) cancel: CancellationToken,
Expand Down Expand Up @@ -654,7 +646,6 @@ impl Tenant {
attached_conf: AttachedTenantConf,
shard_identity: ShardIdentity,
init_order: Option<InitializationOrder>,
tenants: &'static std::sync::RwLock<TenantsMap>,
mode: SpawnMode,
ctx: &RequestContext,
) -> anyhow::Result<Arc<Tenant>> {
Expand Down Expand Up @@ -828,52 +819,6 @@ impl Tenant {
// Remote preload is complete.
drop(remote_load_completion);

let pending_deletion = {
match DeleteTenantFlow::should_resume_deletion(
conf,
preload.as_ref().map(|p| p.deleting).unwrap_or(false),
&tenant_clone,
)
.await
{
Ok(should_resume_deletion) => should_resume_deletion,
Err(err) => {
make_broken(&tenant_clone, anyhow::anyhow!(err), BrokenVerbosity::Error);
return Ok(());
}
}
};

info!("pending_deletion {}", pending_deletion.is_some());

if let Some(deletion) = pending_deletion {
// as we are no longer loading, signal completion by dropping
// the completion while we resume deletion
drop(_completion);
let background_jobs_can_start =
init_order.as_ref().map(|x| &x.background_jobs_can_start);
if let Some(background) = background_jobs_can_start {
info!("waiting for backgound jobs barrier");
background.clone().wait().await;
info!("ready for backgound jobs barrier");
}

let deleted = DeleteTenantFlow::resume_from_attach(
deletion,
&tenant_clone,
preload,
tenants,
&ctx,
)
.await;

if let Err(e) = deleted {
make_broken(&tenant_clone, anyhow::anyhow!(e), BrokenVerbosity::Error);
}

return Ok(());
}

// We will time the duration of the attach phase unless this is a creation (attach will do no work)
let attached = {
let _attach_timer = match mode {
Expand Down Expand Up @@ -931,21 +876,13 @@ impl Tenant {
)
.await?;

let deleting = other_keys.contains(TENANT_DELETED_MARKER_FILE_NAME);
info!(
"found {} timelines, deleting={}",
remote_timeline_ids.len(),
deleting
);
info!("found {} timelines", remote_timeline_ids.len(),);

for k in other_keys {
if k != TENANT_DELETED_MARKER_FILE_NAME {
warn!("Unexpected non timeline key {k}");
}
warn!("Unexpected non timeline key {k}");
}

Ok(TenantPreload {
deleting,
timelines: Self::load_timeline_metadata(
self,
remote_timeline_ids,
Expand Down Expand Up @@ -974,7 +911,6 @@ impl Tenant {
let preload = match (preload, mode) {
(Some(p), _) => p,
(None, SpawnMode::Create) => TenantPreload {
deleting: false,
timelines: HashMap::new(),
},
(None, _) => {
Expand Down Expand Up @@ -2628,7 +2564,6 @@ impl Tenant {
cached_synthetic_tenant_size: Arc::new(AtomicU64::new(0)),
eviction_task_tenant_state: tokio::sync::Mutex::new(EvictionTaskTenantState::default()),
activate_now_sem: tokio::sync::Semaphore::new(0),
delete_progress: Arc::new(tokio::sync::Mutex::new(DeleteTenantFlow::default())),
cancel: CancellationToken::default(),
gate: Gate::default(),
timeline_get_throttle: Arc::new(throttle::Throttle::new(
Expand Down
Loading

1 comment on commit 188797f

@github-actions
Copy link

Choose a reason for hiding this comment

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

3004 tests run: 2878 passed, 0 failed, 126 skipped (full report)


Flaky tests (1)

Postgres 14

  • test_tenant_delete_smoke: release

Code coverage* (full report)

  • functions: 32.5% (6866 of 21131 functions)
  • lines: 50.0% (53380 of 106757 lines)

* collected from Rust tests only


The comment gets automatically updated with the latest test results
188797f at 2024-06-24T12:07:56.600Z :recycle:

Please sign in to comment.