Skip to content

Commit 111ae08

Browse files
goffrieConvex, Inc.
authored andcommitted
Always consider min_{index,document}_snapshot_ts repeatable (#43468)
These timestamps are always repeatable, since the retention workers will never advance them past the persisted max repeatable ts. There is no point re-verifying that they are <= max_repeatable_ts in FollowerRetentionManager, as that incurs an extra database roundtrip (& the current implementation is technically buggy, since we should be reading max_repeatable_ts _after_ the snapshot ts, or else we could spuriously fail the `prior_ts` check; though in practice the two timestamps are kept far enough apart that this would never actually fail). GitOrigin-RevId: c974ca3995bb0713a9662828b3c91dd812d13cf8
1 parent 393a494 commit 111ae08

File tree

2 files changed

+28
-14
lines changed

2 files changed

+28
-14
lines changed

crates/common/src/types/timestamp.rs

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@ pub enum RepeatableReason {
2222
TableSummarySnapshot,
2323
/// ts <= max_ts from persistence, and no Committer is running
2424
IdleMaxTs,
25+
/// ts <= min_index_snapshot_ts or min_document_snapshot_ts from persistence
26+
/// globals
27+
MinSnapshotTsPersistence,
2528
/// ts <= some other RepeatableTimestamp
2629
InductiveRepeatableTimestamp,
2730
/// only in tests

crates/database/src/retention.rs

Lines changed: 25 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ use common::{
100100
GenericIndexName,
101101
IndexId,
102102
PersistenceVersion,
103+
RepeatableReason,
103104
RepeatableTimestamp,
104105
Timestamp,
105106
},
@@ -232,7 +233,7 @@ pub struct LeaderRetentionWorkers {
232233
pub async fn latest_retention_min_snapshot_ts(
233234
persistence: &dyn PersistenceReader,
234235
retention_type: RetentionType,
235-
) -> anyhow::Result<Timestamp> {
236+
) -> anyhow::Result<RepeatableTimestamp> {
236237
let _timer = match retention_type {
237238
RetentionType::Document => latest_min_document_snapshot_timer(),
238239
RetentionType::Index => latest_min_snapshot_timer(),
@@ -251,7 +252,14 @@ pub async fn latest_retention_min_snapshot_ts(
251252
None => Timestamp::MIN,
252253
_ => anyhow::bail!("invalid retention snapshot {min_snapshot_value:?}"),
253254
};
254-
Ok(min_snapshot_ts)
255+
// We have the invariant:
256+
// min_document_snapshot_ts <= min_index_snapshot_ts <= max_repeatable_ts
257+
// and the latter ts is repeatable by definition, so min_snapshot_ts is
258+
// always repeatable.
259+
Ok(RepeatableTimestamp::new_validated(
260+
min_snapshot_ts,
261+
RepeatableReason::MinSnapshotTsPersistence,
262+
))
255263
}
256264

257265
const INITIAL_BACKOFF: Duration = Duration::from_millis(50);
@@ -1592,16 +1600,23 @@ async fn load_snapshot_bounds(
15921600
latest_retention_min_snapshot_ts(persistence, RetentionType::Index).await?;
15931601
let min_document_snapshot_ts =
15941602
latest_retention_min_snapshot_ts(persistence, RetentionType::Document).await?;
1595-
if *repeatable_ts < min_index_snapshot_ts {
1603+
if repeatable_ts < min_index_snapshot_ts {
15961604
anyhow::bail!(snapshot_invalid_error(
15971605
*repeatable_ts,
1598-
min_index_snapshot_ts,
1606+
*min_index_snapshot_ts,
15991607
RetentionType::Index
16001608
));
16011609
}
1610+
if repeatable_ts < min_document_snapshot_ts {
1611+
anyhow::bail!(snapshot_invalid_error(
1612+
*repeatable_ts,
1613+
*min_document_snapshot_ts,
1614+
RetentionType::Document
1615+
));
1616+
}
16021617
Ok(SnapshotBounds {
1603-
min_index_snapshot_ts: repeatable_ts.prior_ts(min_index_snapshot_ts)?,
1604-
min_document_snapshot_ts: repeatable_ts.prior_ts(min_document_snapshot_ts)?,
1618+
min_index_snapshot_ts,
1619+
min_document_snapshot_ts,
16051620
})
16061621
}
16071622

@@ -1665,22 +1680,18 @@ impl<RT: Runtime> RetentionValidator for FollowerRetentionManager<RT> {
16651680
}
16661681

16671682
async fn min_snapshot_ts(&self) -> anyhow::Result<RepeatableTimestamp> {
1668-
let snapshot_ts = new_static_repeatable_recent(self.persistence.as_ref()).await?;
1669-
let latest = snapshot_ts.prior_ts(
1683+
let latest =
16701684
latest_retention_min_snapshot_ts(self.persistence.as_ref(), RetentionType::Index)
1671-
.await?,
1672-
)?;
1685+
.await?;
16731686
let mut snapshot_bounds = self.snapshot_bounds.lock();
16741687
snapshot_bounds.advance_min_snapshot_ts(latest);
16751688
Ok(latest)
16761689
}
16771690

16781691
async fn min_document_snapshot_ts(&self) -> anyhow::Result<RepeatableTimestamp> {
1679-
let snapshot_ts = new_static_repeatable_recent(self.persistence.as_ref()).await?;
1680-
let latest = snapshot_ts.prior_ts(
1692+
let latest =
16811693
latest_retention_min_snapshot_ts(self.persistence.as_ref(), RetentionType::Document)
1682-
.await?,
1683-
)?;
1694+
.await?;
16841695
let mut snapshot_bounds = self.snapshot_bounds.lock();
16851696
snapshot_bounds.advance_min_document_snapshot_ts(latest);
16861697
Ok(latest)

0 commit comments

Comments
 (0)