Skip to content

Commit 2400425

Browse files
ldanilekConvex, Inc.
authored and
Convex, Inc.
committed
[retention] calculate indexes before starting the retention loops (#24237)
there's a race where: 1. indexes calculated at ts 0 2. min_snapshot_ts advances 3. document min_snapshot_ts advances 4. index retention deleter tries to recalculate indexes by walking the log from 0 to the current time, but this is outside of retention. to fix, we should calculate all possible indexes at the current timestamp before starting the workers to advance timestamps GitOrigin-RevId: 1dfee975a9091e79d67677c73114a184bd5b8dbb
1 parent 1c7285c commit 2400425

File tree

1 file changed

+26
-8
lines changed

1 file changed

+26
-8
lines changed

crates/database/src/retention.rs

Lines changed: 26 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,7 @@ use common::{
8686
GenericIndexName,
8787
IndexId,
8888
PersistenceVersion,
89+
RepeatableTimestamp,
8990
TabletIndexName,
9091
Timestamp,
9192
},
@@ -256,7 +257,7 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
256257
// We need to delete from all indexes that might be queried.
257258
// Therefore we scan _index.by_id at min_snapshot_ts before min_snapshot_ts
258259
// starts moving, and update the map before confirming any deletes.
259-
let indexes_at_min_snapshot = {
260+
let mut all_indexes = {
260261
let reader = persistence.reader();
261262
let snapshot_ts =
262263
new_static_repeatable_ts(min_snapshot_ts, reader.as_ref(), &rt).await?;
@@ -280,6 +281,19 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
280281
};
281282
let index_table_id =
282283
index_table_id.ok_or_else(|| anyhow::anyhow!("there must be at least one index"))?;
284+
let mut index_cursor = min_snapshot_ts;
285+
let latest_ts = snapshot_reader.lock().latest_ts();
286+
// Also update the set of indexes up to the current timestamp before document
287+
// retention starts moving.
288+
Self::accumulate_indexes(
289+
persistence.as_ref(),
290+
&mut all_indexes,
291+
&mut index_cursor,
292+
latest_ts,
293+
index_table_id,
294+
follower_retention_manager.clone(),
295+
)
296+
.await?;
283297

284298
let (send_min_snapshot, receive_min_snapshot) = async_channel::bounded(1);
285299
let (send_min_document_snapshot, receive_min_document_snapshot) = async_channel::bounded(1);
@@ -303,9 +317,9 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
303317
bounds_reader.clone(),
304318
rt.clone(),
305319
persistence.clone(),
306-
indexes_at_min_snapshot,
320+
all_indexes,
307321
index_table_id,
308-
min_snapshot_ts,
322+
index_cursor,
309323
follower_retention_manager.clone(),
310324
receive_min_snapshot,
311325
checkpoint_writer,
@@ -979,7 +993,7 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
979993
bounds_reader: Reader<SnapshotBounds>,
980994
rt: RT,
981995
persistence: Arc<dyn Persistence>,
982-
indexes_at_min_snapshot: BTreeMap<IndexId, (GenericIndexName<TableId>, IndexedFields)>,
996+
mut all_indexes: BTreeMap<IndexId, (GenericIndexName<TableId>, IndexedFields)>,
983997
index_table_id: TableIdAndTableNumber,
984998
mut index_cursor: Timestamp,
985999
retention_validator: Arc<dyn RetentionValidator>,
@@ -988,7 +1002,6 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
9881002
snapshot_reader: Reader<SnapshotManager>,
9891003
) {
9901004
let reader = persistence.reader();
991-
let mut all_indexes = indexes_at_min_snapshot;
9921005

9931006
let mut error_backoff = Backoff::new(INITIAL_BACKOFF, *MAX_RETENTION_DELAY_SECONDS);
9941007
let mut min_snapshot_ts = Timestamp::default();
@@ -1020,10 +1033,12 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
10201033
)
10211034
.await?;
10221035
tracing::trace!("go_delete: loaded checkpoint: {cursor:?}");
1036+
let latest_ts = snapshot_reader.lock().latest_ts();
10231037
Self::accumulate_indexes(
10241038
persistence.as_ref(),
10251039
&mut all_indexes,
10261040
&mut index_cursor,
1041+
latest_ts,
10271042
index_table_id,
10281043
retention_validator.clone(),
10291044
)
@@ -1040,10 +1055,12 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
10401055
)
10411056
.await?;
10421057
tracing::trace!("go_delete: finished running delete");
1058+
let latest_ts = snapshot_reader.lock().latest_ts();
10431059
Self::accumulate_indexes(
10441060
persistence.as_ref(),
10451061
&mut all_indexes,
10461062
&mut index_cursor,
1063+
latest_ts,
10471064
index_table_id,
10481065
retention_validator.clone(),
10491066
)
@@ -1279,20 +1296,21 @@ impl<RT: Runtime> LeaderRetentionManager<RT> {
12791296
persistence: &dyn Persistence,
12801297
all_indexes: &mut BTreeMap<IndexId, (GenericIndexName<TableId>, IndexedFields)>,
12811298
cursor: &mut Timestamp,
1299+
latest_ts: RepeatableTimestamp,
12821300
index_table_id: TableIdAndTableNumber,
12831301
retention_validator: Arc<dyn RetentionValidator>,
12841302
) -> anyhow::Result<()> {
12851303
let reader = persistence.reader();
12861304
let mut document_stream = reader.load_documents(
1287-
TimestampRange::greater_than(*cursor),
1305+
TimestampRange::new(*cursor..*latest_ts)?,
12881306
Order::Asc,
12891307
*DEFAULT_DOCUMENTS_PAGE_SIZE,
12901308
retention_validator,
12911309
);
1292-
while let Some((ts, _, maybe_doc)) = document_stream.try_next().await? {
1310+
while let Some((_, _, maybe_doc)) = document_stream.try_next().await? {
12931311
Self::accumulate_index_document(maybe_doc, all_indexes, index_table_id)?;
1294-
*cursor = ts;
12951312
}
1313+
*cursor = *latest_ts;
12961314
Ok(())
12971315
}
12981316
}

0 commit comments

Comments
 (0)