mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-11-14 23:06:59 +00:00
d44445fb295fd4866dc88fcc6923d28b7bed7dca
5483
Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
 discord9
|
d44445fb29 |
Merge remote-tracking branch 'narthex-binary-runtime' into feat/narthex-fused-tree
Bring in #9813's later budget fixes. The public `NativeExecutor` API this branch uses (`try_new`, `push_raw`, `next_chunk`, `finish`) is unchanged, so nothing here needed adjusting: the merge is clean and the tree still builds. Review asked for this: the branch was written against `859b549e70f8` and the runtime has moved on since. Signed-off-by: discord9 <discord9@163.com> |
||
 discord9
|
2f3a34dc6f |
fix(promql): skip accounting ledger for unbudgeted vector merges
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
4b7f7fc3ab |
fix(promql): charge binary buffers at actual allocation boundaries
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
90f08fd0a1 |
fix(promql): scope binary reservations to live scans and output rows
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
9b015bc57d |
fix(promql): charge for the label sets the plan keeps, not for one stream
The occupancy map is the plan's: entries are inserted by whichever partition sees a label set first and no path removes them. The reservation paying for them was the stream's, so the bytes went back when that stream ended while the entries stayed. Measured on a plan with three partitions producing three label sets: each grew 42 bytes, each stream's end returned them, and the pool then read 0 while every entry was still there and still colliding with later partitions. The plan's own drop produced no shrink at all. So a label set's bytes were charged for part of the time and invisible for the rest, and the invisibility grew with the number of label sets a query touches. The charge now belongs to the map it pays for: `LabelSets` holds the entries and the reservation together, `attach` registers it once on the first execution's pool, and it is released when the plan is dropped. `attach` is idempotent because every partition calls it. Re-registering would move a charge the other partitions keep growing, and would give it back when one stream ends while the entries stay - which is the shape of the bug. A regression test states the invariant: after the first partition's stream ends, the pool is still charged for its label set; a second partition with the same set on disjoint points adds no further charge; dropping the plan returns it to zero. Signed-off-by: discord9 <discord9@163.com> |
||
|
discord9
|
d1003ac8fe |
feat(promql): run a bound tree over raw series in one fused node
`FusedSeriesExec` evaluates a compiled `NativeTree` directly over the raw series its child produces, instead of materialising a row batch through the per-operator chain. It binds the time column, the fields and the tags once, checks the input against that binding when the plan is built, and converts the chunks the tree settles back into the statement's output columns. The node's partition count is the child's, one tree per partition. The child must be a raw-series scan that assigns every series whole to one partition: `PerSeries` distribution is what makes a per-partition evaluation equivalent to evaluating the whole series. A zero-partition input is refused; anything else is the child's business. A label set can recur across partitions once `rate` and the map kernels drop `__name__`, so the occupancy map is plan state rather than per-stream. Two series reaching one label set is an error only when their occupied grid points overlap, which is Prometheus' range-query rule; disjoint points are legal and are merged into one occupancy record. The reservation for that map is attached to the first execution's pool and released when the plan is dropped, because a later partition still has to collide with what an earlier one emitted. `occupied_runs` reads a chunk's absolute positions rather than assuming one contiguous run: a sparse evaluated chunk may hold several, and treating the first as the whole range would count the holes as rows. A poll does a bounded amount of work and wakes itself when the budget runs out, so a long run of chunks that settle nothing cannot monopolise the executor thread. Signed-off-by: discord9 <discord9@163.com> |
||
|
discord9
|
859b549e70 |
fix(promql): reserve binary workspace along the shared kernel
Remove the separate matching prepass and reserve current live workspace at the existing kernel allocation boundaries. Release per-sample and per-field scratch after ownership ends; charge actual retained rows, values, histograms and errors. Unbudgeted public and set callers bypass accounting without another computation path. Validate with cargo fmt -p promql -- --check; cargo check -p promql --all-targets; cargo nextest run -p promql --retries 0 --no-fail-fast; cargo clippy -p promql --all-targets -- -D warnings. The public nested Binary test also fails on swapped left/right operands and passes after byte-identical restoration. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
49803e5874 |
test(promql): infer the binary result fixture element type
Drop a redundant complex type annotation flagged by the actual strict clippy run. The collection and literal assertions are unchanged. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
1c77715c9a |
feat(promql): execute vector binary nodes in the native runtime
Checkpoint the vector-vector runtime and single shared matching kernel. Keep addressed input delivery, whole-input buffering, nested composition and error cleanup together. The existing arithmetic/comparison implementation remains the only computation path. Local review checkpoint only, not ready for publication: all-targets check passes; the current run has three new fixture failures. Independent review requires current-live workspace resizing, field-local matched storage and kept-value merge budgeting before approval. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
fc2189f4c4 |
docs(promql): describe the gather test by what this branch does
The doc on `an_oversized_selection_is_refused_by_the_pool` said the kernel has no point limit any more. It does on this branch: the limit is removed on its own branch, which this one does not carry, so the comment described a state that has not arrived. What is true here is the order: the estimate now applies to every gather, so the pool is asked before the kernel is, and the pool's refusal is the one the caller sees. That is worth saying on its own -- a bound in bytes on the path that decides the points, rather than a count that says nothing about what they cost -- and it stays true after the limit goes. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
8b8e3cc66a |
fix(promql): reserve the instant gather's workspace and keep the histogram start
Two findings from the review of the instant node. The gather allocates after the last chunk arrives: the concatenated axis, the descriptors that read it, the two arrays the selection returns, and the rows the builder keeps. Nothing asked the pool for any of it, so with one sample selected by every point of a million-point grid the node took tens of megabytes while its reservation fell to the buffered chunks' size -- nothing at all, on the chunk that ends the series. `gather_bytes` now derives a bound from the values in hand (rows, chunks, segments, fields, points, and the widest histogram row per field, since one row can be selected by every point), and the reservation grows by it before the gather runs. It shrinks back on both the success and the error path, so the workspace never outlives the call. A selection reads an existing sample, so it has to carry that sample's payload. `FloatHistogram` had no `start_timestamp` and `push_to` wrote `None`, which lost it for any histogram that had one: `detect_start_timestamp_reset` reads that field to spot a counter reset, so a window over this output would mis-detect resets. The field is now carried through a view and written back; the kernels that compute a histogram instead of reading one leave it `None`, which is what they produced before. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
3f08b6af51 |
Merge commit '3630ba0488d' into feat/narthex-unified-exec
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> # Conflicts: # src/promql/src/native/execute.rs |
||
|
discord9
|
3630ba0488 |
fix(promql): refuse an interleaved input, and close a label set only for its owner
Two series-level identity holes, both reported against `42ea525`.
The truncation flag followed the last chunk handed over, which is the same as
"is a series open" only while at most one series can be open. A tree with no
window still accepted `Interleaved`, and `a` open, `b` closed, end of input then
passed while `a` never closed. Tracking the open set would need an identity that
a raw chunk in bound-label mode does not have (`metric` is `None`, the key holds
it), so the ordering is refused when the state is built instead, for every tree
shape and not only under a window. The flag's doc now says what it means and why
that is the whole answer.
`History` closed on any series' `is_last`, so with `abs` dropping the metric name
a series that contributed nothing could close a label set another series was
still filling: `a{job="x"}` value, `b{job="x"}` empty close, `a{job="x"}`
continuation was refused with "already closed the label set". Contiguous input
hands over exactly that sequence, so it was not an interleaving artefact. A
history now remembers the series that filled it and closes only for that series'
closing chunk; a series that produced nothing under an open set is passed over,
as it already was under a closed one. The owner is charged as part of the entry.
The ordering check also reads every input node rather than only the first: a tree
with two inputs would otherwise have its second declaration read and dropped.
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com>
|
||
|
discord9
|
73760651f7 |
feat(promql): execute an instant selection as a tree node
The tree could not run `NativeExpr::Instant`: the state builder recorded `not_implemented` and refused the whole tree before reading a chunk, while the compiler already produced the node for a bare selector. A query the plan and the compiler both accept therefore had no runtime path at all. `InstantRuntime` buffers one series to its `is_last` chunk -- the selection kernel reads a complete axis -- then calls the shared `select_instants` once and gathers the rows it picked onto the binding's own grid. Positions come from the grid lattice the kernel was asked for, and a timestamp off that lattice is an error rather than a skipped row. A series that selects nothing still closes, with an empty chunk on the grid; an input that ends inside a series is reported on `finish()`. The node does not record label-set occupancy: it keeps the metric verbatim, so it cannot map two series onto one label set, and the occupancy key drops empty-valued tags -- two genuinely distinct series would be refused as a collision. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
42ea525cc8 |
fix(promql): report a truncated input from a tree with no window
`close` only asked windows whether a series was still open, so a tree of an input alone, or of a map over one, accepted a final chunk without `is_last` and handed the consumer a series that never closed. The input node now follows the series of the chunk it last handed over, and closing such a tree reports the truncation against it, which is the same refusal a window makes for the input it reads. `occupied_runs` read a row the axis names with `expect`, which panics on a `Series` whose fields are shorter than its axis; the fields are public, so that chunk is constructible by a caller. It now returns the misalignment as an error naming the node, the field and both lengths. The chunk protocol says one series closes once, which reads as one close per label set. State that it is per input series: a node that rewrites labels can hand two series over under one label set and both of them close. Also say in `Emitted`'s doc that the table is one node's own, so a label set costs its bytes again in every node that keeps one. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
0e4ff30dd2 |
perf(promql): append a forward-moving chunk to the emitted history
`absorb` rebuilt the whole stored run array on every non-empty chunk. Legal input made that quadratic across chunks with no label collision: one series in `K` chunks, chunk `i` occupying only grid position `2*i`, stores one more run per chunk and moves `K(K+1)/2` of them -- about five billion for a hundred thousand rows. A chunk that starts at or past the end of the last stored run only ever adds to the end, so its runs are appended and the boundary run is joined where it touches. One that can interleave or reach backwards still takes the full one-pass merge, which can split a run. Both leave the runs sorted, disjoint and joined, which is what `Emitted::occupy` looks a position up by. The test records the chunks through `Emitted::occupy` itself and reads the history back from it, covering the forward case, the touching boundary, a hole that has to join both neighbours, and a position before everything stored. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
963b1fe742 |
fix(promql): keep the source queue's charge incremental
The queue's reservation was recounted by walking every queued chunk on each enqueue and each pop. `push_raw` and `push_evaluated` are public and put no bound on how deep the queue gets, so a caller that queues a whole series before pulling it paid `N(N+1)/2` chunk visits to hold `N` chunks -- the same shape of defect this branch fixes in `absorb`. `QueuedCharge` keeps a reference count and the bytes behind each shared allocation: a table or buffer is charged when the first queued chunk reaches it and given back when the last one leaves, so an operation walks the one chunk it names. The count is per chunk rather than per field, because a pop re-walks the same chunk with the same dedupe and must remove exactly the references that were added. A chunk's own struct bytes are counted per chunk (`own_bytes`); only the allocations behind its fields are counted by address, since a struct has no address to dedupe on. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
922c354b98 |
fix(promql): charge the memory the native evaluator actually retains
Four omissions made the reservation blind to most of what a window over native histograms keeps, and the existing incremental-equals-recomputed test could not see any of them because both sides used the same formula. - `FloatHistogram` owns four heap allocations -- both bucket lists, the custom bounds, and the span layout -- that `FieldSamples::bytes` never looked at. A retained 4096-bucket sample is tens of kilobytes against a ledger that saw a struct header. `heap_bytes` counts them, and both the retained sample slice and the histograms a row boxes are charged for what they own. - Queued chunks were estimated as one float per field per row, which is not what a histogram field holds: it holds indices into a shared `Arc<HistogramBatch>` and the buffers behind the Arrow arrays. The figure now walks the samples, and counts a shared table or a shared buffer slice once per pass rather than once per chunk that references it. - `history_bytes` included `size_of::<History>()` while the map slot term charged a `History` per entry as well, so every entry paid two headers. The per-entry figure now covers only the heap the entry owns. - The primary key is charged for the allocation its `Buffer` keeps alive rather than the bytes it shows: a scan slices the key out of a decoded dictionary-values buffer, so a short key can pin a batch-sized allocation. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
9bd5c85a0f |
test(promql): pin the empty-map output count at one
The test checked that every output was empty and that exactly one closed, which a stray extra non-closing empty output would also satisfy. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
e75af676b8 |
refactor(promql): drop the window queue the sparse evaluator never needed
`RangeEvaluator::push` and `push_series` each return `Result<Option<EvaluatedChunk>>`: the evaluator gathers every run it decided into one chunk and hands back at most one. So `pending.extend(decided)` inserted at most one chunk and `take_pending` removed it on the spot, leaving the queue empty after every successful push. That makes the queue dead weight: a `VecDeque` allocation per productive window, an accounting term that is always zero, a second drain site in `pull`, and doc comments claiming a problem the API does not have. The driver's "ask the input again when a window decides nothing" loop was already the whole story. Back to returning the evaluator's own answer, and drop `pending`, `take_pending` and `take_window_output`. Covers the empty-map protocol this port introduces while it is here: an operation that empties every position hands on nothing while its series is open, and one empty chunk when it closes. A non-closing empty chunk would be rejected downstream and a dropped closing one would leave the series unclosed. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
ed6e32d048 |
feat(promql): execute a bound native tree node by node, over sparse evaluated series
Ports the native runtime onto the sparse evaluated-series API: a chunk is addressed by a `TimeAxis` that may hold several runs, so a consumer reads positions through the axis instead of assuming `start + row`. The driver and both node kinds move to that API: - `chunk.range().len()` becomes `chunk.series.time.len()`, the compact row count, in every metric and byte estimate. - `occupied_runs` walks `time.positions()` with one field cursor, so a chunk of several runs records each run where it belongs. A run open across a hole now ends where the hole begins; leaving it open to the last row made two runs look like one and refused a series that occupies the hole. - A window keeps the pieces its evaluator decided in a queue: one push can decide several runs, and reporting only what the push returned stranded the rest until the next push. Queued pieces are charged by `retained_bytes`, and the charge is recomputed when one leaves, so a released piece is not held. - `map_chunk` answers `Option`: an operation that empties every position (`clamp` with `max < min`) yields no chunk at all, unless the chunk closes its series -- a closing chunk holding no row is handed over so a consumer can tell "nothing more here" from "no more input". `RangeEvaluator::retained_bytes` is restored with the three byte helpers it reads: a field's buffers, a series' identity and the pending batch's vectors, all by capacity, and still counted as storage rather than as a peak over a push. Tests read absolute positions from the axis (`decided` no longer adds a row number to a start), and two sparse cases cover what the old reading got wrong: a `Window -> Map -> Window` chain over a chunk of two runs, and a series that occupies the hole between another's runs. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
3ad5f96328 |
feat(promql): compile a parameterised range function (#9743)
* feat(promql): compile a parameterised range function `quantile_over_time(q, x[r])` now compiles: the quantile is bound as a number literal and the range selector is bound by the same path a function without parameters takes. The two shapes share one binder, so the range, the offset, the anchor and the grid cannot drift between them. The quantile is not range-checked. Prometheus binds any value and the kernel answers `-Infinity` below zero and `+Infinity` above one, so refusing it here would be this compiler inventing a rule the engine does not have. A parameter in parentheses binds the same quantile: the planner already looks through the parentheses when it reads a literal, so a query written `quantile_over_time((0.9), x[5m])` keeps working on this path too. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(promql): cover a parenthesised selector of a parameterised function The unparen rule applies to the selector a parameterised function binds, not only to the one a plain range function binds: a query that writes its range selector in parentheses asks for the same tree. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * docs(promql): name the real run-time parameter, and reuse `unparen` The doc said `1 + 1` is computed at run time. It is not: `try_build_literal_expr` folds `Paren`, `Unary` and literal-only `Binary` in the existing planner, so `quantile_over_time(1 - 0.1, x[5m])` and `quantile_over_time(-(0.9), x[5m])` plan today and reach this compiler unfolded. Refusing them is the agreed behaviour for this stage, not a run-time-only case, and the doc now says so; `time()` is the real run-time example. A test pins both folded forms as refused, so implementing folding later is a deliberate change rather than a silent one. `scalar_parameter` open-coded the parenthesis loop `unparen` already implements, so the rule for transparent wrappers now lives in one place. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
44df7a2a1f |
feat(promql): compile a bare selector and an anchored range window (#9736)
* feat(promql): compile a bare selector and an anchored range window The compiler refused a bare instant selector and any `@` anchor, so the two shapes an ordinary query starts from could not reach the tree. It now binds a selector's lookback, offset and output grid into an instant node, and resolves `@ 1234`, `@ start()` and `@ end()` into the millisecond anchor a range window already carries, bound apart from the offset so a shift and an anchor are not folded into one number. A bare selector's own anchor stays refused: an instant node binds a lookback, an offset and a grid and has no field to anchor one. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(promql): tell `@ end()` apart from the last grid point The anchor test used a statement ending at 100s on a 5s step, so its last grid point was also 100s: binding `end()` to the last grid point instead of to the statement's own end would have passed. The end is now a parameter of the statement helper, and a statement ending at 103s pins the two apart, asserting both the anchor and that the grid still stops at 100s. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * docs(promql): write down what an anchored window reads, and cover a pre-epoch anchor An `@`-anchored window reads `(anchor - offset - range, anchor - offset]`, which can lie entirely outside the statement's own range, so whoever binds the scan has to derive it from the window rather than from `stmt.start` and `stmt.end`. A scan bound to the statement alone leaves such a window with no samples, and the query returns empty without an error. That contract was nowhere in the code. The anchor cases only covered non-negative anchors, so a helper that defaulted a pre-epoch anchor to zero would have kept the suite green. `rate(x[5m] @ -1.5)` now pins -1500, which is the `Err(earlier)` branch of the conversion. The `assert_ne!` at the end of the `end()` test was implied by the two equalities above it and could not fail on its own. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
4c7cc8b10b |
feat(promql): compile a statement into a native tree and display it (#9732)
* feat(promql): compile a PromQL statement into a native tree Compile a range selector and its surrounding shape into the native plan nodes introduced earlier, and render the resulting tree through `Display` so an `EXPLAIN` can show it. The display carries each node's operation, its bound window and the fields it reports. Field layout is validated here rather than at execution: a node that declares a different number of fields than it consumes is a planning error, which is what the fixture tests pin. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): strip parentheses the native compiler does not read through Parentheses group; they say nothing about the expression inside them, so `(rate(x[5m]))` asks for exactly what `rate(x[5m])` asks for. The compiler matched the bare shape only and refused the parenthesised one as NotImplemented, rejecting a statement on how it was typed rather than on what it means. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): refuse an ordering a window cannot consume, and stop claiming an executor that does not exist `compile` recorded the scan's ordering without checking it, so `compile(rate(x[5m]), schema, Interleaved)` built a tree that `validate` accepted and the range evaluator was guaranteed to reject at run time: it reads one series to its last chunk before the next. The ordering is refused where it is declared now, and the tree's own window check is the backstop. The tree carries no metric name or matchers, so the contract on `input` decides correctness and was nowhere written: matchers, `__field__` among them, have to be applied before `input` is bound, and `input.fields` is the post-`__field__` set. The display marked a hand-written list of nodes "(no executor)". No executor exists at this point in the stack, so `Window` and `Map` read as runnable while `Instant` was marked as not, although `select_instants` runs in production. The marker is gone; every line reads as the node it is and claims nothing about whether it could run. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): render a grouping as the shape it was bound as The tree now binds an aggregate's grouping as `Grouping::{By, Without}` (#9723), so `label_list(&op.group)` no longer type-checks: the argument is a `Grouping`, and the function takes `&[Arc<str>]`. This is production code, not a test helper, so the merged stack would not build. Rendering on the `By`/ `Without` shape fixes more than the type error: both were printed as `by [... ]`, which would show `by(job)` and `without(job)` — two different queries over the same label list — as the same query. A `without` aggregate now reads `sum without [job] complete`, and `a_without_grouping_does_not_read_as_by` pins it. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
f2f8043fdd |
feat(promql): add the bound native tree and the metric surface it reports through (#9723)
* feat(promql): add the bound tree and batch protocol for native execution A native PromQL evaluation needs a compiled form whose operations are already bound to kernels, fields and grids. Recognising an opportunity from a finished physical plan cannot provide that: the opportunity then depends on how the optimizer happened to arrange a subtree, and every new shape widens a fixed template. `NativeExpr` instead covers input, scalar, instant, window, map, aggregate and binary nodes, each carrying a bound operation and a declared output type. Legality becomes a question about types and data dependencies, which `NativeNode::validate` answers, so a new combination of supported operations is an ordinary tree rather than another matcher. Two properties are checked there rather than assumed: a window's declared input grid must be the grid its input produces, so a subquery's inner grid is consumed as it is instead of being resampled, and an aggregate's output type must match its stage, so intermediate state cannot pass for a query value. `EvaluatedChunk` is the batch a node hands to the next one: an ordinary Rust value, addressed on absolute grid positions, closing a series exactly once. An empty final chunk is still meaningful — it is how a closed prefix is reported, which a consumer needs in order to tell "nothing more here" from "no more input". Nodes are identified by their position in the tree rather than by an id. The position is derived by the one traversal both `DisplayAs` and run-time construction use, so what is shown and what is registered cannot disagree. It stays in the metric name because the collector aggregates by name first, which would otherwise merge two nodes of the same operation. This adds the tree and the batch protocol only. The evaluator's chunked interface, memory reservation and execution are not part of it. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): make the bound tree's type contracts match what each stage consumes Four review findings on the bound tree, all about checks that passed the wrong things. An aggregate's stage decided its output but not its input. Every stage first required its input to be evaluated, which aggregate state is not, so `Final(Partial(...))` was rejected and `Final(Window(...))` was accepted. Each stage now states its pair: `Complete` takes a vector and produces one on the same grid, `Partial` takes a vector and produces state on that grid with the bound grouping, `Final` takes state with the bound grouping and produces a vector on the state's grid. Window and instant selections accepted aggregate state. The cause was asking `is_evaluated()`, which is false both for raw samples and for state, since state has no grid: `Instant(Partial(...))` and a window bound to `input_grid: None` over `Partial(...)` both passed and would have read state as raw samples. Both now match on `NativeType::Samples` first and only then on the grid, so state is rejected by what it is rather than by lacking a grid. Scalars were modelled as a grid-less evaluated sample, and `BoundScalar`'s doc claimed a value varying per grid point is not a scalar. That is wrong for PromQL: `time()` and `scalar(v)` produce a scalar per evaluation time. A scalar is now its own `NativeType::Scalar(grid)`, so a binary operation can tell a broadcast from label matching -- `sum by(job)(rate(x[5m])) + time()` broadcasts rather than matching on the empty label set -- and `Map`, `Aggregate`, `Instant` and `Window` reject it by type. The `time()` kernel itself is not in scope. An input could bind only samples, so an exchange boundary could not bind the state a `Final` aggregate consumes. `BoundInput` holds a `NativeType`, and input nodes check that their output is the same binding rather than the same schema. A peak is the one figure the collector must not add up: it groups metrics by name, clears the partition and sums gauges, so partition peaks of 1024 and 2048 reported 3072, which is neither peak. `PEAK_RESERVED_BYTES` now carries the partition in its name (`native.p0.root.peak_reserved_bytes`), while counts and the other gauges still sum across partitions, where a sum is meaningful. The test that claimed the peak kept its partition only checked a name prefix; it now registers two partitions, runs the real `aggregate_by_name`, and asserts both peaks survive with their own values. The walkthrough's `+1` test was wrong twice over: it took a `Final` aggregate directly over a `Window` -- the case the first finding now rejects -- and applied `MathOp::Abs`, which is not `+1`. It now builds a real binary `+` with a scalar operand. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * refactor(promql): use the evaluator's chunk type for native batches `native/batch.rs` defined its own `EvaluatedChunk` with the same two fields as `promql::evaluated::EvaluatedChunk`. Two definitions of one protocol drift: the evaluator's chunk could not be handed to a native node without translation, and a change to either would have to be mirrored. Use `crate::evaluated::EvaluatedChunk` instead. The type moves here with the evaluator's documentation, which states the same two rules this module did: a grid position is absolute, and `is_last` is the only thing that closes a series. The module keeps its own doc explaining the rules native nodes rely on and re-exports the type, so `native::batch::EvaluatedChunk` still names it. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): reject a node that declares a different field count than it consumes A window and a map both report one value per input field, so they keep the layout of their input. Validation checked that both sides were samples and that the grids matched, but not that the field counts did, so a tree could declare two output fields over a one-field input and still pass `NativeTree::try_new`. The mismatch then surfaced at run time, against a schema the tree was supposed to have validated. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): validate the field layout of every node that declares one Field counts were checked for a window and a map and nothing else, so an instant selection, either aggregate stage and a binary operation could each declare a layout their kernel does not produce and still pass validation, surfacing at run time against a schema the tree was supposed to have validated. Each node is now checked against the count its own kernel produces, which for a binary over two vectors is the shorter side. Grids are compared by their specification, as the kernels compare them, instead of by allocation. A compiler that allocates one grid per window -- `rate(a[5m]) + rate(b[5m])` -- built a tree the runtime would run and validation rejected. Binding identity is still compared by allocation: an input has to report the one binding it was given, which is a different question from what two equal grids mean. A rejected tree is a compiler defect, so it is reported as an internal error rather than as a mistake in the client's statement, which `Plan` would have implied. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): check a window's input ordering and range, and drop the tests that only repeat another module `validate` accepted an interleaved input for a window, although the range evaluator reads one series to its last chunk before it starts the next, so the tree this layer builds was guaranteed to fail at run time. A window now rejects both an interleaved input and a range that is not positive. An instant selection reads one series at a time, so it keeps accepting an interleaved input and now says why. The grid comment claimed the input grid is checked to cover the window's read range. It is not: only a subquery feeds an evaluated input to a window, and this compiler does not accept subqueries yet. The binary operation's catch-all arm is unreachable at run time -- state has no grid and the grid checks return before the match -- but the match still has to cover the state pairs, so they are spelled out and say so instead of claiming a value arrived. Rejection branches had no tests, so deleting any of them kept the suite green. Each now has one. The four tests in `native/batch.rs` repeated the ones in `evaluated.rs` through the re-export, so they could not fail on their own; they are gone, and the coverage stays in `evaluated.rs`. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): bind a grouping as `by` or `without`, not as a label list A label list can only spell `by`: `sum without (instance) (rate(x[5m]))` has no list of group labels, because its key is every label the series has except the ones named. Spelling that as a list would group the query by labels it meant to remove, and the kernel takes both shapes already (`aggregate::Grouping::{By, Without}`), so the tree had no way to say what the engine can already do. State carries its grouping in the same terms now, so a `Final` also refuses state built for the other shape, not only state built from a different label list. The `groups` gauge counts per partition: a `Partial` aggregate sees one group in every partition it reaches, so the summed gauge is the total of per-partition entries rather than distinct groups. Both the metric's doc and the comment that claimed otherwise now say so. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Signed-off-by: discord9 <discord9@users.noreply.github.com> |
||
|
discord9
|
872753e4cd |
refactor(promql): read and write sparse evaluated series (#9750)
* refactor(promql): read and write sparse evaluated series The axis can now say a series holds values at only some grid positions. This makes PromQL use that: the builder stops padding its output, every operator reads positions and rows from the axis instead of deriving one from the other, and the streaming protocol stops treating a run boundary as a delivery boundary. `GridSeriesBuilder::finish` returns one `Series`, or `None` when no retained row holds a value, with the axis built from the runs the retained positions make up. It keeps one buffer per field and slices runs out of it by span, so a series with values at 10 and 900000 on a million-point grid costs two rows, not the distance between them. The queue that cut a series into one piece per run is gone: `take_buffered` hands out everything buffered so far as one `Series` and ends nothing, so a caller that wants a prefix still gets one while the later rows stay on the same logical series. The position cursor resets only in `begin_series`, which is what marks the next logical series; a flush must not rewind it, and a test pins that. Every reader follows. A place that took one start offset and computed a row as `position - start` now asks the axis for the row, or walks positions with a forward cursor per field, or intersects two axes with `intersect_runs` carrying one compact offset per side -- broadcast and vector matching used to compare envelope bounds and subtract starts, which is only correct while every axis is contiguous. Matches stay exhaustive: no wildcard arm was added. The six functions that had grown a `Vec` purely because a result could be segmented return `Option<Series>` again: `absent`, `map_native_histogram`, `map_date`, `map_compare`, `map_arithmetic_series` and `map_grid_scalar`. Each has one output identity per input -- one metric name, one field set, one axis -- and a hole in the result is a property of that axis, not a second identity. `None` means no row survived. Operators that genuinely fan one input into several identities keep returning a `Vec`: aggregation groups, topk, binary and set matching, `merge_fields`, `merge_same_labelset` and the label operators; those reuse one builder per output series and now call `begin_series` explicitly, since `finish` no longer resets. An evaluated series may now be closed by an empty delivery. `push_series` refused any empty input while the evaluator itself produces an empty closing chunk after a valued prefix and for an identity that never produced a value, so a pipeline of two evaluators rejected its own output. An empty chunk that is not the last still decides nothing and stays an error; the last one is a close, whatever it carries. `SeriesState::last_ts` becomes an `Option` so an identity with no position at all has no timestamp to invent. Three walks that were correct but quadratic are fixed here because the shapes changed under them: binary matching rebuilt its two sample cursors for every shared run, so each run re-read every row before it; set matching asked the filter for presence one point at a time and topk asked one member at a time, both through `Samples::sample`, which scans chunk descriptors from the start; and topk ranked every grid point instead of the union of the members' positions. The cost argument is from the code, not measured -- how many samples a walk reads is not observable through the public API -- while the tests pin that the results are unchanged under fragmentation. The per-operator suites asserted the old shapes, so the expectations that said "one piece per run" now say "one series, several runs". Two of them could not have failed before: histogram aggregation was compared through a helper that dropped every non-histogram row, so a padded range and a sparse one compared equal, and the sparse `map_date` case claimed to exercise the cutting path while holding only present samples. Both now assert the exact runs, row counts and alignment, so a return to padded output fails. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * perf(promql): pick a merged row's cursor by slot, not by search `merge_same_labelset` collects the rows of every member of a label group and then merges them position by position. Collecting stores which member each row came from, and the merge read that member's value through a per-member cursor, because a member of several runs must not re-scan its field chunks per row. Between the two it looked the member up again for every row: let member_index = members .iter() .position(|candidate| *candidate == member) .expect("the row names one of the members"); That is a linear search over the group per row, so a group of N members with one disjoint value each pays N(N+1)/2 comparisons just to find its cursors -- on the path a label function takes when it drops the label that kept the series apart, which is the ordinary way several identities become one. The previous shape indexed `series[member]` directly and had no such term. The rows already hold the member; make it hold the member's slot instead. `members` is built in ascending input order, so sorting by slot orders the merge exactly as sorting by member did, and the cursors stay forward-only. No lookup, no map, no change to the merge itself. The regression case is the one that exposes it: many members whose single values sit at distinct positions share a label set once the distinguishing label is dropped, so the collision check never short-circuits and the whole merge runs. It asserts the merged axis carries one row per member with each member's own value, which a wrong slot fails -- checked by forcing the slot to zero, where both this case and the existing disjoint-fields case fail. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): say what a chunk does and does not promise, and compare without folding `EvaluatedChunk` claimed the positions after the last row handed out had not been decided yet, and `range()` claimed it reported how far a series had been evaluated. Neither holds: a push can settle a trailing stretch as empty and hand out nothing for it. What holds is that a position no chunk carries either holds no value or has not been decided, and a consumer cannot tell the two apart, so only `is_last` says a series is complete. `assert_result` folded the series it was given by label set before comparing, so a producer that wrongly cut one label set into two series was folded back together and passed. It now refuses that outright; `fold_pieces` stays for the callers that genuinely fold a stream. `RangeEvaluator::pull` and the `finish` branch that checked it were dead: every push ends in an `emit` that drains the builder, so nothing is ever left to pull and seven assertions said a condition that could not be false. `skipped_and_all_missing_positions_are_the_same_series` pushed `&[None]` where it meant to skip, so both builders received the same calls and the test compared a builder with itself. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
82c7d145b7 |
feat(datatypes): support sparse evaluation-grid axes (#9762)
* feat(datatypes): support sparse evaluation-grid axes A series that holds a value at only some grid positions had no way to say so: its time axis was one contiguous range, and an explicit timestamp list is not the same thing -- it loses the evaluation grid the series is validated against, and cannot answer which grid position a row belongs to. `TimeAxis` already owns positions, length, timestamp access and slicing, and already has a chunked form that keeps several Arrow buffers unflattened. This gives grids the same treatment. `GridSpan` is one run of absolute positions; `GridRanges` is an ordered, non-overlapping list of spans on one shared `EvalGrid`; `TimeAxis::GridRanges` carries it beside the contiguous `TimeAxis::Grid(GridRange)` fast path, which keeps its contract including the zero-copy `timestamps()` slice. The coordinates are fixed by the axis and documented on it. `len()` counts carried rows, not the distance from the first position to the last; `timestamp`, `slice` and `row_of` speak compact rows; `positions()` walks `(absolute position, compact row)` pairs in one pass with one cursor per span; `intersect_runs` merges two run lists without forming their product; `union_positions` merges several axes. Spans are validated at construction -- ordered, non-overlapping, inside the grid, arithmetic checked -- and an axis with no rows still owns its grid, so nothing unwraps a first span to find it. `row_of` is a binary search and `slice` rebuilds through the checked constructor. `Samples` gains a forward `RowCursor` so a walk reads rows in order without scanning chunk descriptors per row. `Series::try_new` accepts the new form under the same rule as the contiguous one: its grid must match `SeriesSchema::eval_grid`, and an explicit axis still may not carry an evaluation grid. Nothing produces a fragmented axis yet. The type and its invariants land first, and every existing reader keeps its behaviour: the places that require a contiguous axis already return an error for anything else, so they reject the new form rather than misreading it. Wiring the readers changes what those places must do with their results, which is why it follows separately. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(datatypes): check the shared grid, keep one constructor, and stop a cursor from poisoning itself `intersect_runs` and `union_positions` walk two axes together but never said they must be addressed on one grid; on an explicit axis every position is 0, so the pairs were silent nonsense. Both now state the precondition and assert it in debug builds. `GridRanges::try_new` accepted a single span, which is not normalized into the one run a `TimeAxis` fast path expects, so a public caller could build a range that reports `is_contiguous()` and still has no grid range. It is now `pub(crate)`; `TimeAxis::from_grid_spans` stays the public way in and keeps normalizing a single span. `Samples::RowCursor::at` drained its iterator on an out-of-range row, so one bad call made every later call answer `None`. It now bound-checks first and leaves the cursor where it was. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
05659b1094 |
perf(promql): hand out evaluated series as chunks instead of one buffered series (#9719)
* perf(promql): hand out evaluated series as chunks instead of one buffered series `RangeEvaluator::push` evaluated every grid point a chunk completed, wrote the rows into its `GridSeriesBuilder`, and then returned `None` for a non-final chunk. A series therefore became visible only on its last chunk: the rows of every earlier chunk stayed buffered in the builder, and a consumer could not release or forward anything until the series ended. That is also why the fused node that evaluates raw samples could emit one batch per series and not per input chunk. `push` now returns the positions it finished as an `EvaluatedChunk`: the evaluated rows plus an `is_last` flag, addressed on the grid by an absolute `GridRange`. A consumer can forward a prefix while the series continues, and `is_last` is the only thing that closes the series. `evaluated.rs` holds that type and the rule it enforces, that a chunk is addressed on a grid. Two protocol points make streaming safe. Ranges of one series only increase, so concatenating the chunks of a series never reorders them. The closing chunk is handed out even when it holds no position, so a series whose remaining grid points have no value still reports that it ended rather than leaving a consumer waiting for input that will not come; previously those series returned `None` and ended silently. `push_series` likewise returns its one closing chunk. Tests fold the chunks of a series back into one series and compare that against the same brute-force reference and Prometheus UDF references as before, so the chunking is checked against the values, not against itself. The fold helper fails on a chunk after the closing chunk, and the golden range-function and mixed-histogram suites, which push one sample per chunk, go through it. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(promql): check the label and position of the chunk that closes a series The fold helper checked the ordering of a chunk only through the rows holding a value, which the grid builder sees. A closing chunk has no rows, so an empty chunk at a wrong position passed. It now compares every chunk's range against the end of the previous one directly, and compares every chunk's metric, since a consumer keys series by their labels and a later chunk with different ones would look like a different series. `an_empty_closing_chunk_is_labelled_like_the_chunks_before_it` covers the label half for `rate`, which drops `__name__`: this also fails if the empty closing chunk stops applying the metadata removal. Also corrects the docs: the range spans the first and last position holding a value, so it is an envelope that may contain missing values, not a report of every position evaluated. `is_last`, not the empty endpoint, says whether the positions after the range are missing or not evaluated yet. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): hand out positions decided to be empty, not only positions holding a value `emit` finished the builder and returned `None` for a non-final chunk when the builder held no valid row. A grid point that was already decided to have no result therefore produced no chunk, and a consumer never learned it was settled: it could not advance its own windows past those points and stayed waiting for input that had already been shown to contribute nothing. The example that fails: inner `rate(x[1s])` on the grid 0..4s. A chunk with samples at -0.5s and 0s decides the point at 0s. A chunk with a sample at 2s decides that 1s has an empty window and 2s a single-sample window -- both missing -- and nothing later can change them, since a sample at 4s lies past both windows. The old code returned `None` there, so an outer `sum_over_time(...[3s:1s])` could not emit its own result for 1s and 2s until the series ended. To report progress, the evaluator now tracks how far it has decided, separately from how far it has values, and the chunk span covers that whole range: - `SeriesState::completed` is the grid index past the last decided position. A point counts as decided when it has been evaluated, when no retained sample reaches its window any more (the loop stops there instead of walking the grid, so the points behind it are reported too), or when the series ends. - `GridSeriesBuilder::cover_from` / `cover_through` widen the builder's span around positions that hold no value, so the finished series reports them as missing instead of trimming them away. The builder's own `finish` still trims to the outermost value for callers that have not decided anything. - `emit` covers `[emitted_end, completed)`, which is empty only when nothing was decided since the last chunk. A series that ends without deciding anything new still closes, on an empty chunk. Tests that pinned the old trimming are updated, and `a_chunk_of_decided_missing_positions_is_handed_out_before_the_series_ends` is the case above: with the sample at 4s not yet arrived, the second push must yield a non-final, all-missing chunk covering points 1 and 2. Folding the three chunks still reproduces the whole grid, so reporting the missing spans costs no values. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * fix(promql): hand out no chunk when a push completes no position `evaluate` opens the builder's span at the first position it has not handed out yet, even when the push settled nothing. `emit` then called `cover_from` and `cover_through` for the decided range, and `finish` built a zero-length series instead of returning `None`, so a consumer received a chunk whose span claimed progress the push had not made. With a grid of 10/20/30ms and a 10ms window, a first chunk holding only `(5ms, 1.0)` reported a zero-length chunk while the first grid point was still undecided. Return `None` when the decided range is empty and this is not the last chunk. The closing chunk is still handed out, empty when nothing is left to report, because `is_last` is what ends a series. `a_push_that_completes_nothing_hands_out_no_chunk` pins that single push, and then checks the closing push still reports every position of the grid. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(promql): fold the batch-path chunk before comparing against the per-window series `a_batch_spanning_fields_and_the_flush_threshold_matches_the_per_window_path` compared the two paths as `Series`, but `push` hands out an `EvaluatedChunk`. Taking `.series` is not enough: a chunk's grid range reports the positions its push decided, which can include leading and trailing positions holding no value, while the reference assertion requires the grid range to be trimmed to the valid rows. The batch path reported `(0, 600)` against the expected `(1, 599)`. Fold the chunk back into one series first, which trims to the valid rows the same way, and compare that. The rebase also removed the `sum` helper along with the test `overlapping_window_spans_chunks`, so the now-unused `floats` helper goes with it. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * chore(promql): let an evaluated chunk be cloned `EvaluatedChunk` holds a `Series` and a bool, both cheap to copy behind their existing ownership, so deriving `Clone` costs nothing. A consumer that has to keep the chunk it matched on, or hand the same chunk to two stages, would otherwise have to destructure and rebuild it. Needed by the native tree, whose bound nodes receive chunks by value and reuse them. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * test(promql): share the owned schema when folding a batch chunk Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
 discord9anddiscord9
|
21e3e10b78 |
perf(promql): evaluate rate and increase over a batch of windows (#9710)
* perf(promql): evaluate rate and increase over a batch of windows `RangeEvaluator` called a boxed `FnMut(FloatWindow) -> Option<f64>` once per grid point per field, building a six-field window struct and appending a row for each. A series of thousands of points therefore made thousands of boxed calls and row appends. The counter functions now take a batch: `BatchKernel` is `FnMut(&FloatWindows, &mut Vec<Option<f64>>)`, selected once outside the loop, and `extrapolated_rate_batch` runs concretely typed per `rate` and `increase`. `FloatWindows` reuses the retained arrays and one sample range per window rather than a copy per window, because a counter function reads only a window's endpoints, sample count and resets. Other functions keep the per-window form and the evaluator falls back to it. This batches the computation only. The results still reach `GridSeriesBuilder` one row at a time, and `flush_batch` reuses the evaluator's row buffer rather than allocating per point. Writing a whole batch as columns needs a column entry point on the builder, which is follow-up work; this commit cannot be read as carrying the whole gain of the experimental branch. A window holding a native histogram sample is decided by the mixed-window rules of `histogram::evaluate`, which read both kinds of sample together and report nothing for a counter over a mixed window. The batch kernel reads the floats alone, so a point whose window holds a histogram flushes what is pending and runs on the per-window path. Without that, a mixed window would report a float rate instead of nothing, and a histogram-only window would lose its histogram result. Tests assert the batch path is the one that ran, since both dispatch shapes produce the same output; a mutation that drops the histogram admission check fails them. Window ranges stay absolute into the retained arrays across a batch, and each accumulated point keeps its own grid index, so a batch spanning a skipped grid point or a chunk boundary still writes the right rows. Signed-off-by: discord9 <discord9@users.noreply.github.com> * test(promql): cover several fields and a mid-evaluation flush in batch window evaluation The batch path was checked only by single-field inputs of at most 20 points, so two of its properties had no test: that each field's windows come from that field's own samples, and that a batch flushes when it fills up before the series ends. Mutating the first -- giving every field field 0's windows -- and `panic!`ing in the second both passed the whole suite. `a_batch_spanning_fields_and_the_flush_threshold_matches_the_per_window_path` pushes 600 points over three fields, each field resetting at a different point, growing at a different rate and missing at a different interval, and compares the batch result against the per-window path on the same input. The gaps are what make the property observable: with every field present at every point the three fields' windows coincide and field 0's spans would be correct by accident, which is how the first form of this test missed the mutation. The histogram admission check also stops scanning every remaining histogram per point and running for functions without a batch kernel: a field's histograms are retained in time order, so the first one ahead of the window head decides, and the check only matters when there is a batch kernel to fall back from. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Co-authored-by: discord9 <discord9@users.noreply.github.com> |
||
|
discord9anddiscord9
|
c500f4697f |
perf(promql): append ordinary raw chunks to the evaluator in bulk (#9709)
* perf(promql): append ordinary raw chunks to the evaluator in bulk `ingest` copied every input sample into `FieldSamples` one at a time. A `RawSeriesChunk` hands the evaluator Arrow buffers `Samples::from_float` wrapped without copying, and `ingest` read them back through `Samples::iter` only to perform, per sample, an enum match, a stale-marker comparison, an overflow-checked offset addition and two `Vec::push` calls. A chunk of null-free float columns read with no offset now appends with `extend_from_slice`. Every other shape keeps the per-sample loop: a shifted timestamp reads different timestamps, a null has no sample, a field mixing native histograms reports `DenseSamplesRef::Mixed` and must keep its windows on the histogram path where the mixed-window rules decide the result, and a chunk whose values contain a stale marker drops its row and needs the per-row decision. The reset index stays per-sample inside the bulk append, so the recorded resets are the same ones the per-sample path records. Tests assert both that the values still match the brute-force reference and that the bulk path is the one that ran, since both paths produce identical output by construction; a mutation that lets a mixed field through the admission check fails them. Signed-off-by: discord9 <discord9@users.noreply.github.com> * refactor(promql): keep the ingest docs with ingest and drop untested guards Three review findings on the bulk-ingest path, none changing behaviour. The doc comment describing `ingest` sat on top of `ingest_in_bulk` after the bulk path was inserted above it, so `ingest` had lost its docs and the bulk function carried documentation for work it does not do. The comment moves back to `ingest`. `ingest_in_bulk` returned `Result<bool>` but never returned `Err`: the two length checks inside it are already done by `check_chunk` for raw input and by `push_series` for evaluated input. It now returns `bool`, and the length checks go with the `Result`. The stale-marker and offset "is not appended in bulk" tests guarded faults the behaviour tests already catch -- the differential and reference comparisons fail if either condition is admitted. The histogram one stays: it is the only test covering that admission rule. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Co-authored-by: discord9 <discord9@users.noreply.github.com> |
||
|
discord9
|
3fca17791c |
perf(promql): index counter resets per field instead of rescanning the window (#9708)
* perf(promql): index counter resets per field instead of rescanning the window The fused evaluator rescanned every window for counter resets, once per grid point. Record the resets of each field as the samples are ingested instead, keyed by an absolute sequence number, and read the ones a window covers with a binary search. Only `rate` and `increase` correct counters, so only they maintain the index. The binary search runs over the field's retained reset index rather than over the window's samples: its input is the remaining slice of resets, so its cost follows the number of retained resets, not the window length. A drop across an intervening native histogram sample is recorded as a reset of two samples that are not neighbours. That is unobservable: a window holding both floats also holds the histogram between them, so it is a mixed window, whose counter functions report nothing; and a window starting after the earlier float excludes a reset on its first sample. Verified with `cargo nextest run -p promql` (453 passed) and `cargo clippy -p promql --all-targets --all-features -D warnings`. Tests assert both that the indexed results match the brute-force scan and that the index is what ran. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * refactor(promql): drop per-field clearing the evaluator never needs Two review findings, both about code that does nothing. `finish_series` cleared every field after `evaluate(None)` returned. That loop is redundant: `evaluate` ends with either every point evaluated -- so `head` moved to the end and `advance` dropped the samples -- or no field holding a sample, and either way the fields are empty. It also did not clear `histograms`, which made it read as though it should. Removing it keeps the suite green, and `push` already documents that an evaluator is not reused after an error. `FieldSamples::clear` existed only for that loop. `overlapping_window_spans_chunks` used a `sum` kernel and never touched resets. Windows spanning chunks are already covered by `evaluator_matches_brute_force_reference`, which chunks randomly, and no mutation was caught only by this test. Its `sum` helper went with it. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Signed-off-by: discord9 <discord9@users.noreply.github.com> |
||
|
discord9
|
04589c175f |
perf(promql): widen the shared extrapolation arithmetic to 64 bits (#9704)
* perf(promql): widen the shared extrapolation arithmetic to 64 bits `extrapolation_factor` is the one place both the float and the native-histogram range paths compute Prometheus' extrapolation. Its three millisecond differences were widened to `i128` and narrowed back with `as f64`; each of those narrowing conversions compiles to a call to the `__floattidf` builtin rather than an instruction, and the function runs once per grid point per series. Use saturating `i64` subtraction instead, which is exact here: a difference too large for `i64` is either a window start below every representable instant, or a distance far past the extrapolation threshold where both values take the same branch. Both callers now share the same arithmetic; neither path carries its own copy. Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> * perf(promql): wrap the extrapolation arithmetic like Go instead of saturating it `extrapolation_factor` widened to `i128`, subtracted, and narrowed back with `as f64`; that compiles to a call to the `__floattidf` builtin once per grid point per series. Doing the subtraction in `i64` removes the calls, but the first form of it clamped with `saturating_sub`, and clamping is not what Go computes: with `eval_ts = i64::MIN + 10`, `range_ms = 100`, `first_t = i64::MIN + 1` and `last_t = i64::MIN + 9`, Prometheus returns 1.625 and the clamped subtraction returns 1.25. `wrapping_sub` is Go's `int64` arithmetic bit for bit, at the same cost. Real timestamps are nowhere near that range, so the two agree on every input a query can produce; the difference only shows up where the old code silently disagreed with Prometheus. Signed-off-by: discord9 <discord9@users.noreply.github.com> Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> --------- Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> Signed-off-by: discord9 <discord9@users.noreply.github.com> |
||
 Lei, HUANG
|
aa4abe554a |
feat(promql): add evaluated series arithmetic and basic aggregates (#9682)
* feat(promql): add evaluated series math kernels Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * feat(promql): add evaluated series scalar arithmetic Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * feat(promql): complete evaluated series math functions Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * feat(promql): add series comparisons and scalar results Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * feat(promql): add sparse evaluated series basic aggregates Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * feat(promql): align time-varying scalar maps on evaluated grids Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): drop metric name in evaluated series math like Prometheus Prometheus' simpleFloatFunc drops __name__ together with __type__ and __unit__. Check abs, ceil, floor, sqrt and round against fixtures generated by the Prometheus v3.15.0 engine. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): drop empty labels with the metric name in series math Prometheus has no empty-valued labels, so a kept empty value would hide a same-labelset collision from the caller. Round shares its calculation with the UDF, so check it only against the Prometheus fixtures. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): align evaluated series map kernels with Prometheus Follow Prometheus v3.15.0 simpleFloatFunc, clamp, VectorscalarBinop and vectorElemBinop: drop __name__, __type__ and __unit__ for math, arithmetic and bool comparisons; use ports of Go math (including the amd64 Log and Exp assembly) for ^, %, atan2 and the transcendental functions; empty a clamp with max < min; keep -0 in sgn and Prometheus' operation order in deg and rad. Fields are compared independently and filtered output is trimmed with GridSeriesBuilder. Check the kernels against fixtures generated by the Prometheus engine and Go math results built for amd64. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): align evaluated series aggregation with Prometheus Follow Prometheus aggregation and generateGroupingLabels: no modifier is by (), without drops __name__, empty label values are absent, sum and avg use Neumaier compensation and avg switches to an incremental mean on overflow. Fields are aggregated independently, so a field without values at a row is null also for count and group. Output is trimmed with GridSeriesBuilder. Check against fixtures generated by the Prometheus engine and drop the main UDAF oracle and the common-function dependency. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): validate whole segments in series comparisons map_compare checked sample types one by one, so a histogram or mixed segment without a present sample passed. Validate segments up front as the other kernels do, check them before an empty clamp, share the empty-label rule with aggregation, and pin exact bits in the deg, rad and incremental avg tests. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * chore: exclude promql golden fixtures from typos Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * docs(promql): point series round at Prometheus funcRound Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): keep Go math ports only where std leaves the tolerance Series math now follows the relative-1e-10 comparison rule: functions use Rust's standard library unless Go's amd64 result differs beyond the tolerance or in overflow and sign. asin, atan, tanh, asinh, acosh and atanh use std; the Exp assembly port is reduced to its overflow edge, and sinh/cosh build on it above 21. Golden and Go math fixtures are compared with almost.Equal. The Go cases are evaluated through the kernels' own per-sample functions, and each retained port must have a case where std fails, so a port that is no longer needed, or a swap back to std, fails a test. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * refactor(promql): validate aggregate input like series maps aggregate_float repeated map's schema, grid-axis and float-segment checks in a different order, so one bad series could fail with a different error category in each. Both now use validate_float_series, and map_math/map_negate/map_arithmetic run it before mapping instead of their own axis check. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * test(promql): pin avg compensation to Prometheus bits With fixtures compared within the tolerance, dropping or misplacing the Kahan compensation in avg only moves the last bits. Three Prometheus cases where it does are compared exactly: compensation on the incremental mean, the separate division of sum and compensation, and rescaling the compensation when avg switches to the incremental mean. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * test(promql): pin the zero sign of clamp and the Neumaier swap Under the fixture tolerance, +0 and -0 compare equal and no case had an addend larger than the running sum, so two branches lost their tests: Go's Max picking +0 over -0, whose sign a later 1/x turns into the sign of an infinity, and the Neumaier swap, without which sum(1, 1e100, 1, -1e100) is 1 instead of 2. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * chore: allow the Unicode category Nd in typos Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * perf(promql): keep aggregate state in fixed-size grid blocks Aggregation kept one B-tree entry and one Vec allocation per populated evaluation point, plus a tree lookup per sample. State is now stored in 64-point blocks keyed by absolute grid index / 64, holding one state per field and point. A block is allocated only once a sample falls in it, so sparse input never expands to the full grid, and each field's run through a block costs one lookup. Points without samples are skipped on output, which also keeps the last block from reaching past the grid. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * perf(promql): drop reserved and empty labels in one rebuild drop_metric_name removed empty-valued labels one at a time, and each MetricName::remove copies the whole label array, so the cost grew with the square of the number of empty labels. Filter the labels once and rebuild the name with MetricName::try_new. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * test(promql): pin that aggregation keeps type and unit labels Prometheus generateGroupingLabels drops only the metric name, unlike math and arithmetic. Nothing failed when grouping dropped __type__ or __unit__. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): return aggregate groups in first-seen order Groups were collected in a BTreeMap and returned sorted by label. Prometheus' rangeEvalAgg appends groups in the order their first input appears, and a nested aggregation folds them in that order: for sum(sum by (g) (m)) over 1e308, -1e308, 1e308 with g = a, c, b, Prometheus gives 1e308 and label order overflows to +Inf. Look groups up by label key and keep them in a Vec. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * test(promql): pin which zero min and max keep Prometheus replaces the min/max accumulator only on a strict comparison, so of +0 and -0 the first is kept. The golden tolerance treats the zeros as equal, and a non-strict comparison passed every test. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> --------- Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> |
||
|
Lei, HUANG
|
50002c6830 |
feat(promql): add evaluated series math kernels (#9672)
* feat(promql): add evaluated series math kernels Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): drop metric name in evaluated series math like Prometheus Prometheus' simpleFloatFunc drops __name__ together with __type__ and __unit__. Check abs, ceil, floor, sqrt and round against fixtures generated by the Prometheus v3.15.0 engine. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * fix(promql): drop empty labels with the metric name in series math Prometheus has no empty-valued labels, so a kept empty value would hide a same-labelset collision from the caller. Round shares its calculation with the UDF, so check it only against the Prometheus fixtures. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * chore: exclude promql golden fixtures from typos Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * docs(promql): point series round at Prometheus funcRound Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * chore: allow the Unicode category Nd in typos Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> * perf(promql): drop reserved and empty labels in one rebuild drop_metric_name removed empty-valued labels one at a time, and each MetricName::remove copies the whole label array, so the cost grew with the square of the number of empty labels. Filter the labels once and rebuild the name with MetricName::try_new. Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> --------- Signed-off-by: Lei, HUANG <ratuthomm@gmail.com> |
||
|
discord9
|
a1052c0c0f |
feat(query): add raw-series preflight and execution surface to query traits (#9661)
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
cbc2a729a5 |
feat(common-meta): add raw-series preflight and execution surface to Datanode trait (#9659)
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
5f0f344908 |
feat(table): scan-bound typed raw series production via SeriesScanExec (#9664)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) **Stacked on #9657** (raw request types). Also requires **#9651** (scanner preflight), **#9655** (frame ordering), **#9656** (recordbatch adapter). ## What's changed and what's your intention? Add scan-bound typed raw series production to `TableScanExec`: - When the scan binding has a `RawSeriesRequest` and preflight passes, produces `SeriesScanExec` that streams typed `RawSeries` frames. - Shares MVCC/projection/filter semantics with the existing RecordBatch path. - Strictly opt-in: absent or declined preflight returns the ordinary plan unchanged. **2 files, +475/-2 production + 1506 test lines.** Note: diff includes cherry-picked commits from #9651 and #9656. Once they merge, the diff will shrink. ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches. |
||
|
discord9
|
18e11944c9 |
feat(promql): add native instant manipulate execution over raw series (#9660)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) **Stacked on #9654** (source materializer). Also requires **#9649** (instant eval) and **#9655** (frame ordering). ## What's changed and what's your intention? Add native instant manipulate execution over raw series streams: - `NativeInstantMapping`: preflighted mapping from typed raw series to query output schema. - `NativeInstantManipulateExec`: ExecutionPlan that consumes `SendableRawSeriesStream`, materializes labels, selects instants, and emits aligned RecordBatches. - `InstantManipulate::try_to_native_execution_plan()`: validates the raw frame schema and constructs the native plan. **5 files, +1830/-0 lines.** No callers wired yet. Note: diff currently includes cherry-picked commits from #9649 and #9655. Once those merge into narthex and the base is updated, the diff will shrink to only the 5 native exec files. ### Verification - `cargo check --all-targets` ✅, `cargo nextest run -p promql` → **357/357 passed** ✅, `cargo clippy -- -D warnings` ✅ ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches. |
||
|
discord9
|
200ed3ce36 |
feat(table): scan-bound typed raw series production via SeriesScanExec
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
f354e3423a |
feat(promql): add native instant manipulate execution over raw series
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
535923629b |
feat(promql): merge ordered raw series inputs
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
83a0d58980 |
feat(query): capture native raw-series routes and extend MergeScan for typed frames (#9663)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) **Stacked on #9662** (native config). Also requires **#9655** (frame ordering metadata). ## What's changed and what's your intention? Add native raw-series route capture and MergeScan typed-frame support: - `CaptureNativeRawRoute`: captures region routing during planning when `experimental_native_promql` is enabled. - `NativeRawSource`: borrows the captured route for native raw-series execution. - Schema identity validation for `greptime.series.` metadata. - Wires `experimental_native_promql` from `QueryEngineState` → `DfQueryPlanner` → `DistExtensionPlanner`. **4 files, +1746/-113 lines.** Note: diff includes cherry-picked commits from #9655. Once it merges, the diff will shrink. ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches. |
||
|
discord9
|
440ecb909b |
feat(common-query): add raw series request binding and preflight types (#9657)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) No dependencies on other open PRs. ## What's changed and what's your intention? Add raw series request types for the named raw-series preflight contract: - `RawSeriesRequest` / `RawSeriesTag`: versioned binding for a single-region raw-series request. - `PreparedNamedRawSeries` + encode/decode: JSON body for the `PREPARE_RAW_SERIES_ACTION` Flight action. Strict serde (`deny_unknown_fields`), version 1 only. Includes wire compatibility fixtures. **7 files, +661 production + wire fixtures.** ### Verification - `cargo check --all-targets` ✅, `cargo nextest run -p common-query` ✅, `cargo clippy -- -D warnings` ✅ ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches, and I have added the `backport-<target>` labels (e.g. `backport-v1.3` targets `release/v1.3`). |
||
|
discord9
|
e0af59f14c |
feat(promql): materialize complete raw source labels (#9654)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) **Depends on #9650** (labels decoder). Must merge after (or be rebased onto) #9650. ## What's changed and what's your intention? Add `promql::source::materialize_raw_labels`: a stream decorator that converts raw series chunks with primary-key-encoded labels into chunks with complete explicit labels. Uses `PrimaryKeyLabelDecoder` (from #9650) to decode dense/sparse primary keys against the scan-bound region metadata snapshot. Rejects schemas with eval grids or empty fields, and requires complete (not projected) label decoding. Output always advertises `RawLabelSource::Explicit`. **5 files, +1216/-0 lines (169 production + 1040 tests + 7 config).** The merge module is intentionally excluded. ### Verification - Compiled and tested with #9650 cherry-picked: `cargo check --all-targets` ✅, `cargo nextest run -p promql` ✅, `cargo clippy -- -D warnings` ✅ - Cannot compile standalone until #9650 merges. ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches, and I have added the `backport-<target>` labels (e.g. `backport-v1.3` targets `release/v1.3`). |
||
 Dennis Zhuang
|
bd9f7dc40b |
docs(promql): say what draining the histograms after an early stop is for
Signed-off-by: Dennis Zhuang <killme2008@gmail.com> |
||
|
Dennis Zhuang
|
328218776d |
fix(promql): follow up the post-merge review of native histogram operators
Signed-off-by: Dennis Zhuang <killme2008@gmail.com> |
||
|
discord9
|
53ef80a516 |
feat(query): capture native raw-series routes and extend MergeScan for typed frames
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
3ef71b5702 |
feat(common-query): add raw series request binding and preflight types
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
7c6595d9f7 |
feat(promql): materialize complete raw source labels
Signed-off-by: discord9 <55937128+discord9@users.noreply.github.com> |
||
|
discord9
|
f8054ab297 |
feat(promql): select instant samples from shared typed fields (#9649)
I hereby agree to the terms of the [GreptimeDB CLA](https://github.com/GreptimeTeam/.github/blob/main/CLA.md). ## Refer to a related PR or issue link (optional) Builds on the shared series types already present on `narthex` (#9598, #9599). This PR targets `narthex`, not `main`, and has no dependency on another open PR. ## What's changed and what's your intention? Extract instant sample selection into `select_instants`, operating on the existing `RawTimestamps` and `SeriesFields`. It returns source row indices and evaluation timestamps without converting the payload into a new representation. Use it in the existing `InstantManipulate` path when timestamps are non-null, strictly increasing milliseconds and the configured fields are Float64. Other inputs retain the existing path, including duplicate timestamps and Histogram columns. Selection preserves lookback boundaries, offsets, stale markers, ordinary NaNs, null samples, and the existing output-point limit. This is one extracted slice of previously local work: **5 files, +732/-3 lines, including tests**. It does not add native planning, scanner/Flight integration, configuration, storage formats, or any new Histogram type. ### Verification - Passed: scoped Rust formatting, `hawkeye format`, `hawkeye check`, `git diff --check`, and normal commit hooks. - Running on this exact extracted branch; results not yet claimed: `cargo check --locked -p promql --all-targets` and `cargo nextest run --locked -p promql --retries 0`. - Independent source review is in progress. Kept as a draft until verification and review are complete. - Full-workspace tests, full-workspace Clippy, and SQLness have not been run for this extracted branch. ## PR Checklist - [x] I have written the necessary rustdoc comments. - [x] I have added the necessary unit tests and integration tests. - [ ] This PR requires documentation updates. - [x] API changes are backward compatible. - [x] Schema or data changes are backward compatible. - [ ] This PR needs to be backported to release branches, and I have added the `backport-<target>` labels (e.g. `backport-v1.3` targets `release/v1.3`). |