From cc599e2aa45c4c17f59859f08fd0a9bd4882088d Mon Sep 17 00:00:00 2001 From: George Talbot Date: Wed, 1 Apr 2026 10:16:58 -0400 Subject: [PATCH 1/3] review: parquet_file singular, proto doc link, fix metastore accessor --- .../src/metastore/postgres/metastore.rs | 87 +++++++++++++------ .../src/split/metadata.rs | 3 +- 2 files changed, 63 insertions(+), 27 deletions(-) diff --git a/quickwit/quickwit-metastore/src/metastore/postgres/metastore.rs b/quickwit/quickwit-metastore/src/metastore/postgres/metastore.rs index 22a3f60dde9..adc3bc1fc19 100644 --- a/quickwit/quickwit-metastore/src/metastore/postgres/metastore.rs +++ b/quickwit/quickwit-metastore/src/metastore/postgres/metastore.rs @@ -2633,45 +2633,80 @@ impl MetastoreService for PostgresqlMetastore { ); // Only delete splits that are marked for deletion + // Match the non-metrics delete_splits pattern: distinguish + // "not found" (warn + succeed) from "not deletable" (FailedPrecondition). const DELETE_SPLITS_QUERY: &str = r#" - DELETE FROM metrics_splits - WHERE - index_uid = $1 - AND split_id = ANY($2) - AND split_state = 'MarkedForDeletion' - RETURNING split_id + WITH input_splits AS ( + SELECT input_splits.split_id, metrics_splits.split_state + FROM UNNEST($2::text[]) AS input_splits(split_id) + LEFT JOIN metrics_splits + ON metrics_splits.index_uid = $1 + AND metrics_splits.split_id = input_splits.split_id + ), + deleted AS ( + DELETE FROM metrics_splits + USING input_splits + WHERE + metrics_splits.index_uid = $1 + AND metrics_splits.split_id = input_splits.split_id + AND NOT EXISTS ( + SELECT 1 FROM input_splits + WHERE split_state IN ('Staged', 'Published') + ) + RETURNING metrics_splits.split_id + ) + SELECT + (SELECT COUNT(*) FROM input_splits WHERE split_state IS NOT NULL) as num_found, + (SELECT COUNT(*) FROM deleted) as num_deleted, + COALESCE( + (SELECT ARRAY_AGG(split_id) FROM input_splits + WHERE split_state IN ('Staged', 'Published')), + ARRAY[]::text[] + ) as not_deletable, + COALESCE( + (SELECT ARRAY_AGG(split_id) FROM input_splits + WHERE split_state IS NULL), + ARRAY[]::text[] + ) as not_found "#; - let deleted_split_ids: Vec = sqlx::query_scalar(DELETE_SPLITS_QUERY) + let (num_found, num_deleted, not_deletable_ids, not_found_ids): ( + i64, + i64, + Vec, + Vec, + ) = sqlx::query_as(DELETE_SPLITS_QUERY) .bind(request.index_uid()) .bind(&request.split_ids) - .fetch_all(&self.connection_pool) + .fetch_one(&self.connection_pool) .await .map_err(|sqlx_error| convert_sqlx_err(&request.index_uid().index_id, sqlx_error))?; - // Log if some splits were not deleted (either non-existent or not - // in MarkedForDeletion state). Delete is idempotent — we don't error - // for missing splits. - if deleted_split_ids.len() != request.split_ids.len() { - let not_deleted: Vec = request - .split_ids - .iter() - .filter(|id| !deleted_split_ids.contains(id)) - .cloned() - .collect(); + if !not_deletable_ids.is_empty() { + let message = format!( + "splits `{}` are not deletable", + not_deletable_ids.join(", ") + ); + let entity = EntityKind::Splits { + split_ids: not_deletable_ids, + }; + return Err(MetastoreError::FailedPrecondition { entity, message }); + } - if !not_deleted.is_empty() { - warn!( - index_uid = %request.index_uid(), - not_deleted = ?not_deleted, - "some metrics splits were not deleted (non-existent or not marked for deletion)" - ); - } + if !not_found_ids.is_empty() { + warn!( + index_uid = %request.index_uid(), + not_found = ?not_found_ids, + "{} metrics splits were not found and could not be deleted", + not_found_ids.len() + ); } + let _ = (num_found, num_deleted); // used by the CTE logic + info!( index_uid = %request.index_uid(), - deleted_count = deleted_split_ids.len(), + num_deleted, "deleted metrics splits successfully" ); Ok(EmptyResponse {}) diff --git a/quickwit/quickwit-parquet-engine/src/split/metadata.rs b/quickwit/quickwit-parquet-engine/src/split/metadata.rs index 57ab6d83eee..0573e67558a 100644 --- a/quickwit/quickwit-parquet-engine/src/split/metadata.rs +++ b/quickwit/quickwit-parquet-engine/src/split/metadata.rs @@ -175,7 +175,8 @@ pub struct MetricsSplitMetadata { /// 0 for newly ingested splits. pub num_merge_ops: u32, - /// RowKeys (sort-key min/max boundaries) as proto bytes. + /// RowKeys (sort-key min/max boundaries) as serialized proto bytes + /// ([`sortschema::RowKeys`](../../quickwit-proto/protos/event_store_sortschema/event_store_sortschema.proto)). /// None for pre-Phase-31 splits or splits without sort schema. pub row_keys_proto: Option>, From 169de714fac3e258a728418b5e83d1d6d02e5e79 Mon Sep 17 00:00:00 2001 From: George Talbot Date: Mon, 6 Apr 2026 13:18:40 -0400 Subject: [PATCH 2/3] style: fix rustfmt nightly comment wrapping in split metadata Co-Authored-By: Claude Opus 4.6 (1M context) --- quickwit/quickwit-parquet-engine/src/split/metadata.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/quickwit/quickwit-parquet-engine/src/split/metadata.rs b/quickwit/quickwit-parquet-engine/src/split/metadata.rs index 0573e67558a..bfef2342f5d 100644 --- a/quickwit/quickwit-parquet-engine/src/split/metadata.rs +++ b/quickwit/quickwit-parquet-engine/src/split/metadata.rs @@ -176,8 +176,9 @@ pub struct MetricsSplitMetadata { pub num_merge_ops: u32, /// RowKeys (sort-key min/max boundaries) as serialized proto bytes - /// ([`sortschema::RowKeys`](../../quickwit-proto/protos/event_store_sortschema/event_store_sortschema.proto)). - /// None for pre-Phase-31 splits or splits without sort schema. + /// ([`sortschema::RowKeys`](../../quickwit-proto/protos/event_store_sortschema/ + /// event_store_sortschema.proto)). None for pre-Phase-31 splits or splits without sort + /// schema. pub row_keys_proto: Option>, /// Per-column zonemap regex strings, keyed by column name. From 21fb72114ff6b5814b8d3a1a82bb60f2dcceb6ac Mon Sep 17 00:00:00 2001 From: George Talbot Date: Mon, 6 Apr 2026 13:30:52 -0400 Subject: [PATCH 3/3] fix: use plain code span for proto reference to avoid broken rustdoc link Co-Authored-By: Claude Opus 4.6 (1M context) --- quickwit/quickwit-parquet-engine/src/split/metadata.rs | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/quickwit/quickwit-parquet-engine/src/split/metadata.rs b/quickwit/quickwit-parquet-engine/src/split/metadata.rs index bfef2342f5d..90cb29d2576 100644 --- a/quickwit/quickwit-parquet-engine/src/split/metadata.rs +++ b/quickwit/quickwit-parquet-engine/src/split/metadata.rs @@ -176,9 +176,8 @@ pub struct MetricsSplitMetadata { pub num_merge_ops: u32, /// RowKeys (sort-key min/max boundaries) as serialized proto bytes - /// ([`sortschema::RowKeys`](../../quickwit-proto/protos/event_store_sortschema/ - /// event_store_sortschema.proto)). None for pre-Phase-31 splits or splits without sort - /// schema. + /// (`sortschema::RowKeys` in `event_store_sortschema.proto`). + /// None for pre-Phase-31 splits or splits without sort schema. pub row_keys_proto: Option>, /// Per-column zonemap regex strings, keyed by column name.