rip out timeline_gc(ignore_lease_deadline=...) flag, not needed anymore

This commit is contained in:
Christian Schwarz
2025-07-04 16:55:18 +02:00
parent 2cd1af2979
commit 9c8d55c529
6 changed files with 21 additions and 93 deletions

View File

@@ -1735,7 +1735,6 @@ pub enum DownloadRemoteLayersTaskState {
#[derive(Debug, Serialize, Deserialize)]
pub struct TimelineGcRequest {
pub gc_horizon: Option<u64>,
pub ignore_lease_deadline: Option<bool>,
}
#[derive(Debug, Clone, Serialize, Deserialize)]

View File

@@ -1147,22 +1147,6 @@ pub(crate) enum LoadConfigError {
NotFound(Utf8PathBuf),
}
pub(crate) enum IgnoreLeaseDeadline {
Default,
No,
Yes,
}
impl From<Option<bool>> for IgnoreLeaseDeadline {
fn from(value: Option<bool>) -> 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<TimelineId>,
horizon: u64,
pitr: Duration,
ignore_lease_deadline: IgnoreLeaseDeadline,
cancel: &CancellationToken,
ctx: &RequestContext,
) -> Result<GcResult, GcError> {
@@ -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,
)

View File

@@ -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.

View File

@@ -406,7 +406,6 @@ async fn gc_loop(tenant: Arc<TenantShard>, cancel: CancellationToken) {
None,
gc_horizon,
tenant.get_pitr_interval(),
crate::tenant::IgnoreLeaseDeadline::Default,
&cancel,
&ctx,
))

View File

@@ -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)

View File

@@ -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
)