make lease more robust

Signed-off-by: Yuchen Liang <yuchen@neon.tech>
This commit is contained in:
Yuchen Liang
2024-06-14 15:03:22 -04:00
parent da55eebc83
commit 723ea86f40
3 changed files with 25 additions and 16 deletions

View File

@@ -181,8 +181,8 @@ impl LsnLease {
pub const DEFAULT_LENGTH: Duration = Duration::from_secs(10 * 60);
/// Checks whether the lease is expired.
pub fn is_expired(&self) -> bool {
SystemTime::now() > self.valid_until
pub fn is_expired(&self, now: &SystemTime) -> bool {
now > &self.valid_until
}
}

View File

@@ -21,7 +21,6 @@ use futures::FutureExt;
use futures::StreamExt;
use pageserver_api::models;
use pageserver_api::models::AuxFilePolicy;
use pageserver_api::models::LsnLease;
use pageserver_api::models::TimelineState;
use pageserver_api::models::TopTenantShardItem;
use pageserver_api::models::WalRedoManagerStatus;
@@ -32,6 +31,7 @@ use remote_storage::DownloadError;
use remote_storage::GenericRemoteStorage;
use remote_storage::TimeoutOrCancel;
use std::fmt;
use std::time::SystemTime;
use storage_broker::BrokerClientChannel;
use tokio::io::BufReader;
use tokio::sync::watch;
@@ -3008,9 +3008,8 @@ impl Tenant {
{
let mut target = timeline.gc_info.write().unwrap();
target
.leases
.retain(|_, lease| !LsnLease::is_expired(lease));
let now = SystemTime::now();
target.leases.retain(|_, lease| !lease.is_expired(&now));
match gc_cutoffs.remove(&timeline.timeline_id) {
Some(cutoffs) => {
@@ -4298,7 +4297,9 @@ mod tests {
)
.await?;
// Keeping everything <= Lsn(0x20) b/c leases: {0/10, 0/20};
// Keeping everything <= Lsn(0x20) b/c leases:
// 0/10: initdb layer
// 0/20: image layer added when creating the timeline.
assert_eq!(res.layers_needed_by_leases, 2);
// Keeping 0/40 b/c it is the latest layer.
assert_eq!(res.layers_not_updated, 1);

View File

@@ -1568,13 +1568,24 @@ impl Timeline {
lsn: Lsn,
_ctx: &RequestContext,
) -> anyhow::Result<LsnLease> {
let lease = LsnLease {
let mut lease = LsnLease {
valid_until: SystemTime::now() + LsnLease::DEFAULT_LENGTH,
};
{
let mut gc_info = self.gc_info.write().unwrap();
gc_info.leases.insert(lsn, lease.clone());
gc_info
.leases
.entry(lsn)
.and_modify(|existing| {
// Insert a lease only if it extends longer than the existing one.
if lease.valid_until > existing.valid_until {
*existing = lease.clone();
} else {
lease = existing.clone();
}
})
.or_insert(lease.clone());
}
Ok(lease)
}
@@ -5036,11 +5047,8 @@ impl Timeline {
// 1. it is older than cutoff LSN;
// 2. it is older than PITR interval;
// 3. it doesn't need to be retained for 'retain_lsns';
// TODO(yuchen): we could consider current retain_lsns as infinite leases.
// 3.5. it does not need to be kept for LSNs holding valid leases (logic is very similar to retain_lsns)
// 4. newer on-disk image layers cover the layer's whole key range
// 4. it does not need to be kept for LSNs holding valid leases (logic is very similar to retain_lsns);
// 5. newer on-disk image layers cover the layer's whole key range
//
// TODO holding a write lock is too agressive and avoidable
let mut guard = self.layers.write().await;
@@ -5091,7 +5099,7 @@ impl Timeline {
}
}
// 3.5 Is there a valid lease that requires us to keep this layer?
// 4. Is there a valid lease that requires us to keep this layer?
if let Some(lsn) = &min_lsn_with_valid_lease {
if &l.get_lsn_range().start <= lsn {
debug!(
@@ -5104,7 +5112,7 @@ impl Timeline {
}
}
// 4. Is there a later on-disk layer for this relation?
// 5. Is there a later on-disk layer for this relation?
//
// The end-LSN is exclusive, while disk_consistent_lsn is
// inclusive. For example, if disk_consistent_lsn is 100, it is