Skip to content

Commit 4a719ce

Browse files
committed
Add missing is_deleting() guards and reconcile stuck tombstones
Retention (get/put), hot tier (put/get/delete), and the distributed get_stats handler could still act on a stream mid-deletion since they never checked is_deleting() after resolving it. Add the same guard used by the other stream endpoints to each of them. Also extend sync_all_streams to periodically scan the tombstone directory itself, not just resident streams: if a background deletion finishes but the final tombstone-clear call fails, the stream is already gone from memory and storage, so the existing per-stream self-heal never sees it again. Retrying delete_stream against an already-empty prefix is a cheap no-op, so this converges the tombstone without waiting for a node restart.
1 parent 04fc9e7 commit 4a719ce

3 files changed

Lines changed: 75 additions & 12 deletions

File tree

‎src/handlers/http/logstream.rs‎

Lines changed: 30 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -303,10 +303,12 @@ pub async fn get_retention(
303303
return Err(StreamNotFound(stream_name.clone()).into());
304304
}
305305

306-
let retention = PARSEABLE
307-
.get_stream(&stream_name, &tenant_id)?
308-
.get_retention()
309-
.unwrap_or_default();
306+
let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
307+
if stream.is_deleting() {
308+
return Err(StreamNotFound(stream_name.clone()).into());
309+
}
310+
311+
let retention = stream.get_retention().unwrap_or_default();
310312
Ok((web::Json(retention), StatusCode::OK))
311313
}
312314

@@ -327,15 +329,18 @@ pub async fn put_retention(
327329
return Err(StreamNotFound(stream_name).into());
328330
}
329331

332+
let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
333+
if stream.is_deleting() {
334+
return Err(StreamNotFound(stream_name).into());
335+
}
336+
330337
PARSEABLE
331338
.storage
332339
.get_object_store()
333340
.put_retention(&stream_name, &retention, &tenant_id)
334341
.await?;
335342

336-
PARSEABLE
337-
.get_stream(&stream_name, &tenant_id)?
338-
.set_retention(retention);
343+
stream.set_retention(retention);
339344

340345
Ok((
341346
format!("set retention configuration for log stream {stream_name}"),
@@ -527,6 +532,10 @@ pub async fn put_stream_hot_tier(
527532

528533
let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
529534

535+
if stream.is_deleting() {
536+
return Err(StreamNotFound(stream_name).into());
537+
}
538+
530539
if stream.get_stream_type() == StreamType::Internal {
531540
return Err(StreamError::Custom {
532541
msg: "Hot tier can not be updated for internal stream".to_string(),
@@ -600,6 +609,13 @@ pub async fn get_stream_hot_tier(
600609
return Err(StreamNotFound(stream_name.clone()).into());
601610
}
602611

612+
if PARSEABLE
613+
.get_stream(&stream_name, &tenant_id)
614+
.is_ok_and(|stream| stream.is_deleting())
615+
{
616+
return Err(StreamNotFound(stream_name.clone()).into());
617+
}
618+
603619
let Some(hot_tier_manager) = GLOBAL_HOTTIER.get() else {
604620
return Err(StreamError::HotTierNotEnabled(stream_name));
605621
};
@@ -635,11 +651,13 @@ pub async fn delete_stream_hot_tier(
635651
return Err(StreamNotFound(stream_name).into());
636652
}
637653

638-
if PARSEABLE
639-
.get_stream(&stream_name, &tenant_id)?
640-
.get_stream_type()
641-
== StreamType::Internal
642-
{
654+
let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
655+
656+
if stream.is_deleting() {
657+
return Err(StreamNotFound(stream_name).into());
658+
}
659+
660+
if stream.get_stream_type() == StreamType::Internal {
643661
return Err(StreamError::Custom {
644662
msg: "Hot tier can not be deleted for internal stream".to_string(),
645663
status: StatusCode::BAD_REQUEST,

‎src/handlers/http/modal/query/querier_logstream.rs‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,13 @@ pub async fn get_stats(
250250
return Err(StreamNotFound(stream_name.clone()).into());
251251
}
252252

253+
if PARSEABLE
254+
.get_stream(&stream_name, &tenant_id)
255+
.is_ok_and(|stream| stream.is_deleting())
256+
{
257+
return Err(StreamNotFound(stream_name.clone()).into());
258+
}
259+
253260
let query_map = web::Query::<HashMap<String, String>>::from_query(req.query_string())
254261
.map_err(|_| StreamError::InvalidQueryParameter(STATS_DATE_QUERY_PARAM.to_string()))?;
255262

‎src/storage/object_storage.rs‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1501,6 +1501,44 @@ pub fn sync_all_streams(joinset: &mut JoinSet<Result<(), ObjectStorageError>>) {
15011501
handle,
15021502
);
15031503
}
1504+
1505+
// Reconcile tombstones left behind by a deletion whose bulk delete
1506+
// succeeded but whose final tombstone-clear call failed (e.g. a
1507+
// transient network error). That stream is already gone from both
1508+
// this node's memory and from storage by the time that happens, so
1509+
// the resident-stream loop above never sees it again -- only a scan
1510+
// of the tombstone directory itself can find it. Same
1511+
// is_deletion_owner restriction as above: only a node that can
1512+
// actually run the physical delete should retry it.
1513+
if PARSEABLE.options.mode != Mode::Ingest {
1514+
let object_store = object_store.clone();
1515+
let tenant_id = tenant_id.clone();
1516+
joinset.spawn_on(
1517+
async move {
1518+
match list_tombstoned_streams(object_store.as_ref(), &tenant_id).await {
1519+
Ok(stream_names) => {
1520+
for stream_name in stream_names {
1521+
// Still resident: already handled by the
1522+
// per-stream loop above, don't double-spawn.
1523+
if PARSEABLE.streams.contains(&stream_name, &tenant_id) {
1524+
continue;
1525+
}
1526+
// delete_stream against an already-empty
1527+
// prefix is a cheap no-op, so this only ever
1528+
// repeats the tombstone clear until it
1529+
// succeeds -- safe to retry every interval.
1530+
spawn_stream_deletion(stream_name, tenant_id.clone());
1531+
}
1532+
}
1533+
Err(e) => error!(
1534+
"failed to list tombstoned streams while syncing tenant {tenant_id:?}: {e}"
1535+
),
1536+
}
1537+
Ok(())
1538+
},
1539+
handle,
1540+
);
1541+
}
15041542
}
15051543
}
15061544

0 commit comments

Comments
 (0)