Repository navigation
Conversation
Motivation: `quickwit index ingest --force` documents that the commit is forced after the last document is sent. The REST client added the `commit=force` (or `wait_for`) query parameter to every batch request instead, so each batch (e.g. ~8 MB with --batch-size-limit 8MB) forced a commit and published its own small split, causing extra uploads, metastore operations and merge work. Approach: in QuickwitClient::ingest, set the `commit` query parameter only when the batch is the last one. Earlier batches use the default (auto) commit. The commit timeout was already applied only to the last batch, so both now use the same condition. Validation: added test_ingest_endpoint_force_commit_only_on_last_batch, which ingests three single-document batches with CommitType::Force and asserts the commit parameter is [None, None, "force"]. With the fix disabled it fails (all three requests carried commit=force); with the fix it passes. `cargo test -p quickwit-rest-client` passes (12 tests) and `cargo clippy -p quickwit-rest-client --tests` is clean. I did not run an end-to-end ingest against a live node; the change is confined to request construction in the client, which the test covers. Report: quickwit-oss#6615 Signed-off-by: Pujitha Paladugu <10557236+pujitha24@users.noreply.github.com> Assisted-by: claude-sonnet-5-5 (via Claude Code)
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 70dd8eb73f
ℹ️ About Codex in GitHub
Codex has been enabled to automatically 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 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
| } | ||
| CommitType::Force => { | ||
| query_params.insert("commit", "force"); | ||
| if is_last_batch { |
There was a problem hiding this comment.
Preserve commit semantics across multiple ingest shards
When an index is configured or scaled to multiple ingest-v2 shards, successive HTTP batches are not ordered through one log: RoutingEntry::pick_node samples eligible ingesters afresh for each request (quickwit/quickwit-ingest/src/ingest_v2/routing_table.rs:86-105,122-143), and a force marker is appended only to the shard selected for that request (quickwit/quickwit-ingest/src/ingest_v2/mrecordlog_utils.rs:58-73); WaitFor similarly tracks only that request's shard position (quickwit/quickwit-ingest/src/ingest_v2/workbench.rs:160-166). Making every earlier batch Auto therefore lets a batch on shard A remain unpublished when the final batch on shard B is published and this call returns, despite the CLI help promising that --wait/--force waits for all documents (quickwit/quickwit-cli/src/index.rs:140-152). Keep per-batch barriers for v2 or add a source-wide final barrier before applying this optimization.
Useful? React with 👍 / 👎.
Description
quickwit index ingest --forceforced a commit on every batch instead of only the last one, producing many tiny splits. The REST client now sets thecommitquery parameter (force/wait_for) only on the last batch; earlier batches use the default auto commit. Adds a regression test.How was this PR tested?
Added
test_ingest_endpoint_force_commit_only_on_last_batch(three batches, assertscommitis[None, None, "force"]). It fails without the fix (all three carriedcommit=force) and passes with it.cargo test -p quickwit-rest-clientpasses (12 tests);cargo clippy -p quickwit-rest-client --testsis clean. No end-to-end run against a live node.Fixes #6615