Repository navigation
Proposal: add live tail - #6899
Proposal: add live tail #6899nico34638 wants to merge 6 commits into
Conversation
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. |
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 91c8f0ec7a
ℹ️ 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".
91c8f0e to
d7e75ee
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: d7e75ee904
ℹ️ 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".
| let live_tail_doc_mapper_opt = self | ||
| .live_tail_hub | ||
| .is_active(subrequest.index_uid()) | ||
| .then(|| doc_mapper.clone()); |
There was a problem hiding this comment.
Check tail subscribers at WAL publication time
When a persist call reaches this check before a tail subscribes but finishes document validation and the WAL append after open_live_tail has returned, this cached None prevents the later publish even though the document was persisted after the stream opened. This race occurs on busy existing shards independently of the documented new-shard gossip delay and silently loses an in-flight batch, so retain the cheap batch/mapper clone and decide whether to publish after the append instead of freezing subscriber activity before the intervening awaits.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Nice catch, that was a real race. We now decide whether to publish after the WAL write, not before validation. Fixed
| use quickwit_common::metrics::exponential_buckets; | ||
| use quickwit_metrics::{LazyCounter, LazyHistogram, lazy_counter, lazy_histogram}; | ||
|
|
||
| pub(crate) static LIVE_TAIL_DOCS_PUBLISHED: LazyCounter = lazy_counter!( |
There was a problem hiding this comment.
Regenerate the inventory for live-tail metrics
For CI runs touching this Rust file, the metrics-inventory workflow includes quickwit/**/*.rs and compares the generated inventory byte-for-byte (.github/workflows/metrics-inventory.yml:47-50,66-73). The new quickwit_live_tail_* metrics are absent from quickwit/quickwit-metrics-inventory/metrics.yaml, so that job will generate a diff and fail; run make metrics-inventory and commit the updated inventory.
Useful? React with 👍 / 👎.
| let event = match message_result?.message { | ||
| Some(live_tail_message::Message::Hits(hits)) => hits_event(index_id, hits)?, | ||
| Some(live_tail_message::Message::Dropped(dropped)) => LiveTailEvent::Dropped { | ||
| reason: dropped.reason(), |
There was a problem hiding this comment.
Reject unspecified live-tail drop reasons
When a malformed or newer ingester sends a LiveTailDropped with reason 0 or an unknown numeric value, Prost's reason() maps it to Unspecified, and this line exposes "unspecified" to SSE although the public contract permits only lagged and rate_limited. Treat this protocol state as an error so the forwarder reports and reconnects instead of silently changing the API shape.
AGENTS.md reference: AGENTS.md:L21-L22
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
it's handled the same way as empty messages, as an error that makes the stream reopen.
97e1499 to
9f59c89
Compare
|
@codex review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 9f59c89d41
ℹ️ 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".
| use crate::metrics::LIVE_TAIL_DOCS_PUBLISHED; | ||
|
|
||
| /// Number of batches buffered per index before slow subscribers start lagging. | ||
| const DEFAULT_CHANNEL_CAPACITY: usize = 64; |
There was a problem hiding this comment.
Bound the hub buffer by bytes instead of batches
When a matching tail client stops reading from a busy index, its session eventually stops draining this receiver and the hub retains up to 64 whole DocBatchV2 values. With the default 10 MiB ingest request limit, that is roughly 640 MiB of raw document data for one index, and the default session limit permits the same accumulation across several indexes, so a slow client can exhaust an ingester's memory; bound this queue by bytes rather than persist-call count.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
True, a client that stops reading could hold a lot of memory. There's now a 64 MiB cap per index; past that, batches are skipped and show up as lagged.
| self.rate_limiter | ||
| .lock() | ||
| .expect("lock should not be poisoned") | ||
| .consume(num_allowed_docs); |
There was a problem hiding this comment.
Don't charge oversized documents against the rate budget
When an oversized matching document precedes normal matches and the requested budget is small, this consumes quota for the oversized document before the later size check drops it. For example, with max_docs_per_sec=1 and [oversized, small], the first document is reported too_large, the second is reported rate_limited, and no hit is sent even though zero documents used the send budget; skip or refund oversized matches and continue selecting sendable documents before consuming quota.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Good catch, oversized docs no longer count against the budget.
| grpc_max_message_num_bytes | ||
| .saturating_sub(HITS_MESSAGE_OVERHEAD_NUM_BYTES) | ||
| .min(TARGET_HITS_PAYLOAD_NUM_BYTES) |
There was a problem hiding this comment.
Negotiate the coordinator's gRPC receive limit
When role-specific node configs differ and the REST/coordinator node has a smaller grpc.max_message_size than the ingester, this cap is computed only from the ingester's limit while live_tail_client configures Tonic decoding from the coordinator's limit. A default ingester can therefore send a near-1 MiB chunk that a coordinator configured at the valid 1 MB minimum rejects, causing repeated stream errors and missed documents; fresh evidence after the earlier size fix is that OpenLiveTailRequest still carries no receiver limit to negotiate.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
the coordinator now sends its limit and the ingester uses the smaller one.
| continue; | ||
| } | ||
| // Ingesters serialize valid JSON objects: this is a violated invariant, not a client error. | ||
| let doc = serde_json::from_slice(line).map_err(|error| { |
There was a problem hiding this comment.
Reject non-object hit documents
If a malformed or newer ingester places a valid JSON scalar or array in docs_ndjson, deserialization into JsonValue succeeds and the coordinator publishes it in docs, even though the protocol and local producer require JSON objects. Check that each value is an object and fail the stream so the protocol violation is reported and reconnected instead of silently changing the SSE shape.
AGENTS.md reference: AGENTS.md:L21-L22
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
Makes sense, it's now treated as a protocol error. fix
Description
When something breaks, you want to see logs as they arrive, like tail -f. In Quickwit, a log is only searchable once its split is published, which can take from 30s to a few minutes. That's too slow to follow a deploy or debug a live issue.
This PR adds
GET /api/v1/{index_id}/tail?query=.... It streams the matching documents as Server-Sent Events as soon as an ingester has persisted them, before they are indexed and committed.flowchart LR subgraph ingester["Each ingester"] P[persist] -->|"valid docs, after the WAL write<br/>(only while someone tails)"| HUB[LiveTailHub<br/>broadcast per index] HUB --> S[LiveTailSession<br/>filter + rate limit] end subgraph rest["Any REST node"] C[Coordinator] end S -->|gRPC stream| C C -->|SSE| CL[Client]The documents are taken as early as possible: in the ingester, right after they're written to the WAL. When someone is tailing an index, the ingester pushes each batch into an in-memory channel. Nothing is copied, and when nobody is tailing, nothing happens at all. Since a WAL position is only written once, there's no replay to worry about.
Each tail session reads that channel, and every 250ms indexes the new docs into a small in-RAM Tantivy index to run the query. So the query behaves exactly like a search. Matches are rate limited and streamed back over gRPC.
The node that receives the request asks chitchat which ingesters have open shards for the index, opens a stream to each, and forwards everything to the client as SSE. The list is refreshed every 5s.
Scope of this first version:
Limits are in the node config and can be overridden: