diff --git a/libs/pageserver_api/src/models.rs b/libs/pageserver_api/src/models.rs index 5f36e8d2b7..16545364c1 100644 --- a/libs/pageserver_api/src/models.rs +++ b/libs/pageserver_api/src/models.rs @@ -1735,7 +1735,6 @@ pub enum DownloadRemoteLayersTaskState { #[derive(Debug, Serialize, Deserialize)] pub struct TimelineGcRequest { pub gc_horizon: Option, - pub ignore_lease_deadline: Option, } #[derive(Debug, Clone, Serialize, Deserialize)] diff --git a/pageserver/src/tenant.rs b/pageserver/src/tenant.rs index 362ec599fb..70a4120c71 100644 --- a/pageserver/src/tenant.rs +++ b/pageserver/src/tenant.rs @@ -1147,22 +1147,6 @@ pub(crate) enum LoadConfigError { NotFound(Utf8PathBuf), } -pub(crate) enum IgnoreLeaseDeadline { - Default, - No, - Yes, -} - -impl From> for IgnoreLeaseDeadline { - fn from(value: Option) -> Self { - match value { - None => IgnoreLeaseDeadline::Default, - Some(false) => IgnoreLeaseDeadline::No, - Some(true) => IgnoreLeaseDeadline::Yes, - } - } -} - impl TenantShard { /// Yet another helper for timeline initialization. /// @@ -3094,7 +3078,6 @@ impl TenantShard { target_timeline_id: Option, horizon: u64, pitr: Duration, - ignore_lease_deadline: IgnoreLeaseDeadline, cancel: &CancellationToken, ctx: &RequestContext, ) -> Result { @@ -3120,23 +3103,7 @@ impl TenantShard { return Ok(GcResult::default()); } - // Skip GC if we're within lease deadline. - // - // Rust & Python tests set single-digit second gc_period and/or - // do immediate gc via mgmt API. The lease deadline is hard-coded to - // 10min, which would make most if not all tests skip GC here. - // Rust tests could in theory tokio::time::advance, but Python - // tests have no such options. - // Since lease deadline is a crutch we hopefully eventually replace - // with durable leases, take a shortcut here and skip lease deadline check - // for all tests. - // Cf https://databricks.atlassian.net/browse/LKB-92?focusedCommentId=6722329 - let ignore_lease_deadline = match ignore_lease_deadline { - IgnoreLeaseDeadline::Default => cfg!(test) || cfg!(feature = "testing"), - IgnoreLeaseDeadline::No => false, - IgnoreLeaseDeadline::Yes => true, - }; - if !ignore_lease_deadline && conf.is_gc_blocked_by_lsn_lease_deadline() { + if conf.is_gc_blocked_by_lsn_lease_deadline() { info!("Skipping GC because lsn lease deadline is not reached"); return Ok(GcResult::default()); } @@ -6054,11 +6021,14 @@ pub(crate) mod harness { let tenant = Arc::new(TenantShard::new( TenantState::Attaching, self.conf, - AttachedTenantConf::try_from(self.conf, LocationConf::attached_single( - self.tenant_conf.clone(), - self.generation, - ShardParameters::default(), - )) + AttachedTenantConf::try_from( + self.conf, + LocationConf::attached_single( + self.tenant_conf.clone(), + self.generation, + ShardParameters::default(), + ), + ) .unwrap(), self.shard_identity, Some(walredo_mgr), @@ -6730,7 +6700,6 @@ mod tests { Some(TIMELINE_ID), 0x10, Duration::ZERO, - IgnoreLeaseDeadline::Default, &CancellationToken::new(), &ctx, ) @@ -6844,7 +6813,6 @@ mod tests { Some(TIMELINE_ID), 0x10, Duration::ZERO, - IgnoreLeaseDeadline::Default, &CancellationToken::new(), &ctx, ) @@ -6905,7 +6873,6 @@ mod tests { Some(TIMELINE_ID), 0x10, Duration::ZERO, - IgnoreLeaseDeadline::Default, &CancellationToken::new(), &ctx, ) @@ -6940,7 +6907,6 @@ mod tests { Some(TIMELINE_ID), 0x10, Duration::ZERO, - IgnoreLeaseDeadline::Default, &CancellationToken::new(), &ctx, ) @@ -7222,14 +7188,7 @@ mod tests { // this doesn't really need to use the timeline_id target, but it is closer to what it // originally was. let res = tenant - .gc_iteration( - Some(timeline.timeline_id), - 0, - Duration::ZERO, - IgnoreLeaseDeadline::Default, - &cancel, - ctx, - ) + .gc_iteration(Some(timeline.timeline_id), 0, Duration::ZERO, &cancel, ctx) .await?; assert_eq!(res.layers_removed, 0, "this never removes anything"); @@ -7847,14 +7806,7 @@ mod tests { // Perform a cycle of flush, and GC tline.freeze_and_flush().await?; tenant - .gc_iteration( - Some(tline.timeline_id), - 0, - Duration::ZERO, - IgnoreLeaseDeadline::Default, - &cancel, - &ctx, - ) + .gc_iteration(Some(tline.timeline_id), 0, Duration::ZERO, &cancel, &ctx) .await?; } @@ -7945,14 +7897,7 @@ mod tests { tline.freeze_and_flush().await?; tline.compact(&cancel, EnumSet::default(), &ctx).await?; tenant - .gc_iteration( - Some(tline.timeline_id), - 0, - Duration::ZERO, - IgnoreLeaseDeadline::Default, - &cancel, - &ctx, - ) + .gc_iteration(Some(tline.timeline_id), 0, Duration::ZERO, &cancel, &ctx) .await?; } @@ -8288,14 +8233,7 @@ mod tests { ) .await?; tenant - .gc_iteration( - Some(tline.timeline_id), - 0, - Duration::ZERO, - IgnoreLeaseDeadline::Default, - &cancel, - &ctx, - ) + .gc_iteration(Some(tline.timeline_id), 0, Duration::ZERO, &cancel, &ctx) .await?; } } @@ -9516,7 +9454,6 @@ mod tests { Some(TIMELINE_ID), 0, Duration::ZERO, - IgnoreLeaseDeadline::Default, &CancellationToken::new(), &ctx, ) diff --git a/pageserver/src/tenant/mgr.rs b/pageserver/src/tenant/mgr.rs index 2888fdeac0..be18b40862 100644 --- a/pageserver/src/tenant/mgr.rs +++ b/pageserver/src/tenant/mgr.rs @@ -842,8 +842,11 @@ impl TenantManager { // take our fast path and just provide the updated configuration // to the tenant. tenant.set_new_location_config( - AttachedTenantConf::try_from(self.conf, new_location_config.clone()) - .map_err(UpsertLocationError::BadRequest)?, + AttachedTenantConf::try_from( + self.conf, + new_location_config.clone(), + ) + .map_err(UpsertLocationError::BadRequest)?, ); Some(FastPathModified::Attached(tenant.clone())) @@ -2365,14 +2368,7 @@ impl TenantManager { #[allow(unused_mut)] let mut result = tenant - .gc_iteration( - Some(timeline_id), - gc_horizon, - pitr, - crate::tenant::IgnoreLeaseDeadline::from(gc_req.ignore_lease_deadline), - &cancel, - &ctx, - ) + .gc_iteration(Some(timeline_id), gc_horizon, pitr, &cancel, &ctx) .await; // FIXME: `gc_iteration` can return an error for multiple reasons; we should handle it // better once the types support it. diff --git a/pageserver/src/tenant/tasks.rs b/pageserver/src/tenant/tasks.rs index e4890efdb0..954dd38bb4 100644 --- a/pageserver/src/tenant/tasks.rs +++ b/pageserver/src/tenant/tasks.rs @@ -406,7 +406,6 @@ async fn gc_loop(tenant: Arc, cancel: CancellationToken) { None, gc_horizon, tenant.get_pitr_interval(), - crate::tenant::IgnoreLeaseDeadline::Default, &cancel, &ctx, )) diff --git a/test_runner/fixtures/pageserver/http.py b/test_runner/fixtures/pageserver/http.py index d60fc34fa3..d9037f2d08 100644 --- a/test_runner/fixtures/pageserver/http.py +++ b/test_runner/fixtures/pageserver/http.py @@ -695,7 +695,6 @@ class PageserverHttpClient(requests.Session, MetricsGetter): tenant_id: TenantId | TenantShardId, timeline_id: TimelineId, gc_horizon: int | None, - ignore_lease_deadline: bool | None = None, ) -> dict[str, Any]: """ Unlike most handlers, this will wait for the layers to be actually @@ -708,7 +707,7 @@ class PageserverHttpClient(requests.Session, MetricsGetter): ) res = self.put( f"http://localhost:{self.port}/v1/tenant/{tenant_id}/timeline/{timeline_id}/do_gc", - json={"gc_horizon": gc_horizon, "ignore_lease_deadline": ignore_lease_deadline}, + json={"gc_horizon": gc_horizon}, ) log.info(f"Got GC request response code: {res.status_code}") self.verbose_error(res) diff --git a/test_runner/regress/test_readonly_node.py b/test_runner/regress/test_readonly_node.py index 98877ad7f9..5612236250 100644 --- a/test_runner/regress/test_readonly_node.py +++ b/test_runner/regress/test_readonly_node.py @@ -203,9 +203,7 @@ def test_readonly_node_gc(neon_env_builder: NeonEnvBuilder): layers_guarded_before_gc = get_layers_protected_by_lease( client, shard, env.initial_timeline, lease_lsn=lease_lsn ) - gc_result = client.timeline_gc( - shard, env.initial_timeline, 0, ignore_lease_deadline=False - ) + gc_result = client.timeline_gc(shard, env.initial_timeline, 0) layers_guarded_after_gc = get_layers_protected_by_lease( client, shard, env.initial_timeline, lease_lsn=lease_lsn )