Add create-only split staging mode - #6714
Conversation
Expose create-only staging semantics in the metastore API and use them during reconciliation so existing split rows are preserved across retries and concurrent writers.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 40a4629f2d
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| quickwit.common.IndexUid index_uid = 1; | ||
| string split_metadata_list_serialized_json = 2; | ||
| // Create-only mode: insert missing rows without upserting an existing split row. | ||
| bool create_only = 3; |
There was a problem hiding this comment.
During a rolling or mixed-version deployment, an older metastore silently ignores unknown protobuf field 3 and executes the existing upsert path, so a new recovery client can overwrite the authoritative split row while receiving a successful response. This semantic mode needs a version-safe RPC or capability gate before callers can rely on it, or an explicitly enforced metastore-first upgrade procedure documented for this protocol change.
AGENTS.md reference: AGENTS.md:L23-L24
Useful? React with 👍 / 👎.
| WHERE splits.split_id = excluded.split_id | ||
| AND splits.split_state = 'Staged' | ||
| AND NOT $11 |
There was a problem hiding this comment.
When concurrent create-only batches contain the same already-existing split IDs in different orders, this remains an ON CONFLICT DO UPDATE statement, and PostgreSQL locks each conflicting row even though the final predicate is false. The workers can therefore acquire different first locks and deadlock, causing one transaction to abort and consume the metastore client's retry budget even though both requests should be conflict no-ops. Use a create-only query with ON CONFLICT DO NOTHING instead.
Useful? React with 👍 / 👎.
| if self.splits.contains_key(split_metadata.split_id()) { | ||
| return Ok(()); |
There was a problem hiding this comment.
When a file-backed create-only request contains only split IDs that already exist, this helper returns the same result as an insertion, and stage_splits consequently returns MutationOccurred::Yes. That makes mutate rewrite the entire index file via put_index for a logical no-op, so repeated recovery retries incur unnecessary storage writes and can fail solely because storage is temporarily unavailable. Return whether an insertion occurred and aggregate that result so an all-skipped batch uses MutationOccurred::No.
Useful? React with 👍 / 👎.
1 participant
Context
Reconciliation may re-stage a split after a retry or while another reconciliation worker is handling the same split. The existing
stage_splitssemantics upsert an already-staged row, so a later request can overwrite metadata written by the first worker. In particular, this can replace the row that represents the currently authoritative recovery candidate.What this changes
Adds a
create_onlyflag toStageSplitsRequest. When enabled, staging inserts only missing split rows and leaves any existing row unchanged. This makes recovery staging idempotent and prevents retries or concurrent reconciliation writers from clobbering an existing split.The default remains unchanged (
create_only: false), preserving the normal upsert behavior for existing callers. The behavior is implemented for both file-backed and PostgreSQL metastores, with coverage for create-only staging.Testing
cargo test -p quickwit-metastore --lib --tests