Make stream deletion asynchronous, resumable across restarts

docker-compose-distributed-test-with-kafka.yaml

@@ -30,7 +30,7 @@ services:
 
   # minio
   minio:
-    image: minio/minio:RELEASE.2025-02-03T21-03-04Z
+    image: quay.io/minio/minio:RELEASE.2025-02-03T21-03-04Z
     entrypoint:
       - sh
       - -euc

docker-compose-distributed-test.yaml

@@ -4,7 +4,7 @@ networks:
 services:
   # minio
   minio:
-    image: minio/minio:RELEASE.2025-02-03T21-03-04Z
+    image: quay.io/minio/minio:RELEASE.2025-02-03T21-03-04Z
     entrypoint:
       - sh
       - -euc

docker-compose-test-with-kafka.yaml

@@ -3,7 +3,7 @@ networks:
 
 services:
   minio:
-    image: minio/minio:RELEASE.2025-02-03T21-03-04Z
+    image: quay.io/minio/minio:RELEASE.2025-02-03T21-03-04Z
     entrypoint:
       - sh
       - -euc

docker-compose-test.yaml

@@ -3,7 +3,7 @@ networks:
 
 services:
   minio:
-    image: minio/minio:RELEASE.2025-02-03T21-03-04Z
+    image: quay.io/minio/minio:RELEASE.2025-02-03T21-03-04Z
     entrypoint:
       - sh
       - -euc

src/handlers/http/ingest.rs

@@ -256,6 +256,16 @@ pub async fn setup_otel_stream(
     let mut time_partition = None;
     // Validate stream compatibility
     if let Ok(stream) = PARSEABLE.get_stream(&stream_name, &tenant_id) {
+        // Unlike plain JSON ingest() and post_event(), this path doesn't go
+        // through validate_stream_for_ingestion, so it needs its own check:
+        // a stream flagged deleting must reject writes rather than staging
+        // data that will never be uploaded (sync_all_streams skips a
+        // deleting stream's staged files) nor cleaned up (the delete
+        // handler's own staging cleanup already ran before this write
+        // recreated the directory).
+        if stream.is_deleting() {
+            return Err(PostError::StreamBeingDeleted(stream_name.clone()));
+        }
         match log_source {
             LogSource::OtelLogs => {
                 // For logs, reject if stream is metrics or traces

src/handlers/http/logstream.rs

@@ -28,7 +28,10 @@ use crate::rbac::Users;
 use crate::rbac::role::Action;
 use crate::stats::{Stats, event_labels_date, storage_size_labels_date};
 use crate::storage::retention::Retention;
-use crate::storage::{ObjectStoreFormat, StreamInfo, StreamType};
+use crate::storage::{
+    ObjectStoreFormat, StreamInfo, StreamType,
+    object_storage::{spawn_stream_deletion, stream_json_path, to_bytes, tombstone_path},
+};
 use crate::tenants::TenantNotFound;
 use crate::utils::actix::extract_session_key_from_req;
 use crate::utils::get_tenant_id_from_request;
@@ -47,50 +50,117 @@ use itertools::Itertools;
 use serde_json::{Value, json};
 use std::fs;
 use std::sync::Arc;
+use tokio::sync::Mutex;
 use tracing::{Instrument, warn};
 
+// Shared between put_stream and delete: without it, a concurrent create/
+// update could read is_deleting()=false right before delete() flips it and
+// writes the tombstone, then go on to write fresh stream data that the
+// background deletion job -- already committed to running by that point --
+// would sweep up from underneath it.
+pub static CREATE_STREAM_LOCK: Mutex<()> = Mutex::const_new(());
+
 pub async fn delete(
     req: HttpRequest,
     logstream: Path<String>,
 ) -> Result<impl Responder, StreamError> {
     let stream_name = logstream.into_inner();
     // Error out if stream doesn't exist in memory, or in the case of query node, in storage as well
     let tenant_id = get_tenant_id_from_request(&req);
-    if !PARSEABLE
-        .check_or_load_stream(&stream_name, &tenant_id)
+
+    // Fetched once, up front: every step below this point is either
+    // infallible or best-effort, so nothing after this line can bail out
+    // with "stream not found" partway through an already-durably-started
+    // deletion.
+    let stream = {
+        // Scoped to just this check-through-tombstone-write window:
+        // everything after is already guarded by is_deleting()/
+        // is_tombstoned() checks on the read/write paths.
+        let _guard = CREATE_STREAM_LOCK.lock().await;
+
+        if !PARSEABLE
+            .check_or_load_stream(&stream_name, &tenant_id)
+            .await
+        {
+            return Err(StreamNotFound(stream_name).into());
+        }
+
+        let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+
+        // Flip the in-memory guard before any `.await` point: check_or_load_stream's
+        // resident-stream fast path doesn't itself consult is_tombstoned, so a
+        // concurrent request on this node could otherwise slip through in the
+        // window between the tombstone becoming durable and this flag being set.
+        stream.mark_deleting();
+
+        let objectstore = PARSEABLE.storage.get_object_store();
+
+        // Durable marker first: if the process crashes anywhere after this
+        // point, restart-recovery resumes the deletion instead of silently
+        // leaving the stream half-deleted with no record of it.
+        if let Err(e) = objectstore
+            .put_object(
+                &tombstone_path(&stream_name, &tenant_id),
+                to_bytes(&()),
+                &tenant_id,
+            )
+            .await
+        {
+            // Nothing durable happened -- undo the in-memory flag so the stream
+            // isn't left permanently blocked by a transient write failure.
+            stream.clear_deleting();
+            return Err(e.into());
+        }
+
+        stream
+    };
+
+    let objectstore = PARSEABLE.storage.get_object_store();
+
+    // Best-effort: makes the stream vanish from listings almost
+    // immediately. Not fatal if it fails -- is_deleting()/is_tombstoned()
+    // checks already block reads and writes regardless of whether this file
+    // is gone yet.
+    if let Err(e) = objectstore
+        .delete_object(&stream_json_path(&stream_name, &tenant_id), &tenant_id)
         .await
     {
-        return Err(StreamNotFound(stream_name).into());
+        warn!(
+            "failed to eagerly delete stream.json for {stream_name}, will be removed with the rest of the prefix: {e}"
+        );
     }
 
-    let objectstore = PARSEABLE.storage.get_object_store();
-
-    // Delete from storage
-    objectstore.delete_stream(&stream_name, &tenant_id).await?;
     // Delete from staging
-    let stream_dir = PARSEABLE.get_or_create_stream(&stream_name, &tenant_id);
-    if let Err(err) = fs::remove_dir_all(&stream_dir.data_path) {
+    if let Err(err) = fs::remove_dir_all(&stream.data_path) {
         warn!(
             "failed to delete local data for stream {} with error {err}. Clean {} manually",
             stream_name,
-            stream_dir.data_path.to_string_lossy()
+            stream.data_path.to_string_lossy()
         )
     }
 
+    // Best-effort: the tombstone is already durable and the stream is
+    // already flagged, so a hot-tier cleanup failure must not turn an
+    // already-accepted deletion into an error response.
     if let Some(hot_tier_manager) = GLOBAL_HOTTIER.get()
         && hot_tier_manager.check_stream_hot_tier_exists(&stream_name, &tenant_id)
-    {
-        hot_tier_manager
+        && let Err(e) = hot_tier_manager
             .delete_hot_tier(&stream_name, &tenant_id)
-            .await?;
+            .await
+    {
+        warn!("failed to delete hot tier for stream {stream_name}: {e}");
     }
 
-    // Delete from memory
-    PARSEABLE.streams.delete(&stream_name, &tenant_id);
-    stats::delete_stats(&stream_name, "json", &tenant_id)
-        .unwrap_or_else(|e| warn!("failed to delete stats for stream {}: {:?}", stream_name, e));
+    // Scheduled only once every other cleanup step above has run, so the
+    // background job (which clears the tombstone on completion) can't race
+    // ahead of them and let a stream recreated under this name get swept up
+    // by leftover local/hot-tier cleanup that's still in flight.
+    spawn_stream_deletion(stream_name.clone(), tenant_id.clone());
 
-    Ok((format!("log stream {stream_name} deleted"), StatusCode::OK))
+    Ok((
+        format!("log stream {stream_name} deletion started"),
+        StatusCode::ACCEPTED,
+    ))
 }
 
 pub async fn list(req: HttpRequest) -> Result<impl Responder, StreamError> {
@@ -209,6 +279,7 @@ pub async fn put_stream(
     let stream_name = logstream.into_inner();
     let tenant_id = get_tenant_id_from_request(&req);
 
+    let _guard = CREATE_STREAM_LOCK.lock().await;
     PARSEABLE
         .create_update_stream(req.headers(), &body, &stream_name, &tenant_id)
         .await?;
@@ -232,10 +303,12 @@ pub async fn get_retention(
         return Err(StreamNotFound(stream_name.clone()).into());
     }
 
-    let retention = PARSEABLE
-        .get_stream(&stream_name, &tenant_id)?
-        .get_retention()
-        .unwrap_or_default();
+    let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+    if stream.is_deleting() {
+        return Err(StreamNotFound(stream_name.clone()).into());
+    }
+
+    let retention = stream.get_retention().unwrap_or_default();
     Ok((web::Json(retention), StatusCode::OK))
 }
 
@@ -256,15 +329,18 @@ pub async fn put_retention(
         return Err(StreamNotFound(stream_name).into());
     }
 
+    let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+    if stream.is_deleting() {
+        return Err(StreamNotFound(stream_name).into());
+    }
+
     PARSEABLE
         .storage
         .get_object_store()
         .put_retention(&stream_name, &retention, &tenant_id)
         .await?;
 
-    PARSEABLE
-        .get_stream(&stream_name, &tenant_id)?
-        .set_retention(retention);
+    stream.set_retention(retention);
 
     Ok((
         format!("set retention configuration for log stream {stream_name}"),
@@ -456,6 +532,10 @@ pub async fn put_stream_hot_tier(
 
     let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
 
+    if stream.is_deleting() {
+        return Err(StreamNotFound(stream_name).into());
+    }
+
     if stream.get_stream_type() == StreamType::Internal {
         return Err(StreamError::Custom {
             msg: "Hot tier can not be updated for internal stream".to_string(),
@@ -529,6 +609,13 @@ pub async fn get_stream_hot_tier(
         return Err(StreamNotFound(stream_name.clone()).into());
     }
 
+    if PARSEABLE
+        .get_stream(&stream_name, &tenant_id)
+        .is_ok_and(|stream| stream.is_deleting())
+    {
+        return Err(StreamNotFound(stream_name.clone()).into());
+    }
+
     let Some(hot_tier_manager) = GLOBAL_HOTTIER.get() else {
         return Err(StreamError::HotTierNotEnabled(stream_name));
     };
@@ -564,11 +651,13 @@ pub async fn delete_stream_hot_tier(
         return Err(StreamNotFound(stream_name).into());
     }
 
-    if PARSEABLE
-        .get_stream(&stream_name, &tenant_id)?
-        .get_stream_type()
-        == StreamType::Internal
-    {
+    let stream = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+
+    if stream.is_deleting() {
+        return Err(StreamNotFound(stream_name).into());
+    }
+
+    if stream.get_stream_type() == StreamType::Internal {
         return Err(StreamError::Custom {
             msg: "Hot tier can not be deleted for internal stream".to_string(),
             status: StatusCode::BAD_REQUEST,

src/handlers/http/modal/ingest/ingestor_logstream.rs

@@ -24,17 +24,23 @@ use actix_web::{
     web::{Json, Path},
 };
 use bytes::Bytes;
+use tokio::sync::Mutex;
 use tracing::warn;
 
 use crate::option::Mode;
 use crate::{
     catalog::remove_manifest_from_snapshot,
     handlers::http::logstream::error::StreamError,
     parseable::{PARSEABLE, StreamNotFound},
-    stats,
     utils::get_tenant_id_from_request,
 };
 
+// Shared between put_stream and delete: without it, a concurrent create/
+// update could read is_deleting()=false right before delete() flips it,
+// then go on to write fresh stream data into a directory delete() is about
+// to (or just did) remove.
+static CREATE_STREAM_LOCK: Mutex<()> = Mutex::const_new(());
+
 pub async fn retention_cleanup(
     req: HttpRequest,
     stream_name: Path<String>,
@@ -76,8 +82,14 @@ pub async fn delete(
 ) -> Result<impl Responder, StreamError> {
     let stream_name = stream_name.into_inner();
     let tenant_id = get_tenant_id_from_request(&req);
-    // Delete from staging
-    let stream_dir = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+
+    let stream_dir = {
+        let _guard = CREATE_STREAM_LOCK.lock().await;
+        // Delete from staging
+        let stream_dir = PARSEABLE.get_stream(&stream_name, &tenant_id)?;
+        stream_dir.mark_deleting();
+        stream_dir
+    };
 
     // delete staging only for ingest server or standalone server
     // else skip
@@ -91,12 +103,16 @@ pub async fn delete(
         )
     }
 
-    // Delete from memory
-    PARSEABLE.streams.delete(&stream_name, &tenant_id);
-    stats::delete_stats(&stream_name, "json", &tenant_id)
-        .unwrap_or_else(|e| warn!("failed to delete stats for stream {}: {:?}", stream_name, e));
-
-    Ok((format!("log stream {stream_name} deleted"), StatusCode::OK))
+    // Not removed from memory here: this node doesn't run the background
+    // deletion job, so it doesn't know when the underlying prefix is
+    // actually gone. The entry is reaped once `sync_all_streams` notices
+    // the tombstone has cleared (see its is_deleting()/is_tombstoned()
+    // self-heal check) -- until then, `is_deleting()` keeps rejecting
+    // ingestion for this stream with a clear "being deleted" error.
+    Ok((
+        format!("log stream {stream_name} deletion started"),
+        StatusCode::ACCEPTED,
+    ))
 }
 
 pub async fn put_stream(
@@ -106,6 +122,7 @@ pub async fn put_stream(
 ) -> Result<impl Responder, StreamError> {
     let stream_name = stream_name.into_inner();
     let tenant_id = get_tenant_id_from_request(&req);
+    let _guard = CREATE_STREAM_LOCK.lock().await;
     PARSEABLE
         .create_update_stream(req.headers(), &body, &stream_name, &tenant_id)
         .await?;