mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-11-14 06:46:56 +00:00
discord9
f73a27d779
* feat(promql): own raw series identities at the repartition boundary A raw frame's is_last closes one producer's stream, not the series: the same stored primary key may still arrive from another input. Routing such frames through the evaluated path therefore emits several partial series where a consumer expects one. Teach SeriesRepartitionExec to group raw identities. SeriesRouting selects the grouping key and try_new_with_routing takes an explicit policy, rejecting the combinations a frame's content cannot honour: an evaluated frame may not use RawPrimaryKey, a raw frame may not use CanonicalLabels, and a raw frame must declare explicit labels. Raw routing keys on the complete stored primary key rather than on labels so two pieces of one physical key always meet in one owner, where a disagreement about labels is rejected instead of being split across owners that would each look consistent on its own. A raw identity is published only after every input reaches normal EOF, unlike an evaluated identity, which is complete once each input either contributed it or ended. merge_raw_identity orders the pieces by timestamp, collapses equal duplicates, rejects conflicting ones and emits one closing chunk. The reconstruction's pool charge covers the two parallel row vectors, the timestamps and the worst-case per-row sample slices that alternating contributions produce. This adds the capability only; no node is constructed with raw routing yet. Signed-off-by: discord9 <discord9@163.com> * test(promql): cover raw identity merging and routing source/merge covers the pure merge: interleaved ordering, duplicate collapse versus bit-level conflict, key and label rejection, histogram property comparison and shape mismatch. The router needs its own proof, because the readiness rule and the identity key are what a consumer depends on. A raw child whose partitions are scripted delivers them: one gated input that has already handed over every chunk but whose stream is still open blocks publication until it ends; identical labels with different primary keys stay apart; pieces of one key that disagree on labels fail; and one key with conflicting samples at one timestamp surfaces an error rather than dropping data. The gate holds back only the end of the stream, so the first test distinguishes "every input contributed" from "every input ended". Swapping the raw readiness rule for the evaluated one makes it fail. Signed-off-by: discord9 <discord9@163.com> * docs(promql): correct the cross-partition merge boundary The guide said native execution does not merge one logical series split across partitions. That stopped being true when the repartition owner began finalizing an exact identity's fragments: the merge now happens at that boundary, and the remaining limit is a logical series split across regions. Signed-off-by: discord9 <discord9@163.com> * fix(promql): charge raw chunks their slice and publish complete identities Two problems in the raw path, both about holding memory longer than the data requires. A frame batch decodes into many chunks that are slices of the same Arrow buffers. Charging each chunk the whole allocation charged one batch's timestamps once per chunk: 64 series of 64 rows retained 4.2 MiB for a 32 KiB timestamp buffer, 129 times the real size, and the reservations live until the identity is published. raw_chunk_slice_bytes charges the slice each chunk actually owns, using the same slice-aware accounting the histogram path already used. The same fixture now retains 95 KiB. A raw identity also waited for every input to reach EOF even when every input had already handed over its complete chain for it. read_raw_chains hands a run of chunks over atomically once is_last arrives, and an input cannot contribute the same key twice, so a contribution is final for that input. Readiness is now the same rule for both contents: every input has contributed the identity or ended. The previous test asserted that a fully contributed identity must still wait for an unrelated input's EOF, which pinned the conservative rule as if it were required. It is replaced by two tests, one for each direction: an identity whose inputs all contributed publishes without waiting, and an identity with an input that has contributed nothing waits. Reverting the readiness rule to the old one fails the first and leaves the second passing. Signed-off-by: discord9 <discord9@163.com> * fix(promql): charge histogram indices and steady the raw accounting tests A histogram field is an index array naming rows of a shared table. Charging only the table missed the indices, and a query over few distinct histograms holds mostly indices: 4096 positions naming one row is 16 KiB of indices against a table of a few hundred bytes. Both the histogram and the mixed branch now charge the index array's slice, as the evaluated path already does. The two accounting tests waited by sampling a fixed number of idle ticks. Under load the input task may not have run yet, so the sample reads zero and the test fails for a reason that has nothing to do with what it measures. Withdrawing the index charge reproduces the failure, so the test still discriminates after the wait was made deterministic. Signed-off-by: discord9 <discord9@163.com> * test(promql): gate the producer so the failure cannot drain unclaimed queues The retention this test reads is not guaranteed across `start` and `claim`. `start` spawns the producer before it returns, and a failure drains every queue nobody has claimed yet, releasing the charges those queues held. A producer that reaches its error first therefore leaves the test claiming two empty queues, and "queued output chains retain memory before drain" can never become true. That is the shape the suite now hits on this machine: the baseline commit fails this test in about ten seconds when the suite runs concurrently with another one, while the same test written with the gate passes every time. The producer now waits on a switch before its error, and the test claims both queues, waits for the retained charge, and only then opens the switch. The assertion keeps its meaning because nothing can drain the queues before the test owns them. This is a test fix. The production cleanup it exposed is correct behaviour: a queue no consumer claimed should not keep holding memory. Signed-off-by: discord9 <discord9@163.com> * feat(promql): route raw frames by a match key A vector operation between two raw sources needs the series of one match key in one partition, and the node could not give it that: raw frames were restricted to key routing, so `100 * (1 - a / b)` could not be planned at all. The restriction rested on an inference the code made when it started a read: the routing policy named the frame content, so only one policy could mean raw. That inference stops holding once a raw frame may be routed by a match key, and the content is now read from the child's own declaration instead. Choosing a destination and deciding a fragment's identity are separate jobs. A match key picks the partition by hashing the labels the frame already carries -- raw frames are required to declare them -- while the stored key still decides which identity a fragment belongs to. Both keep one key's fragments in one partition, because one key always decodes to one label set, so merging is unaffected. Canonical labels stay refused for raw frames: they include the metric name, which is not a match key. Signed-off-by: discord9 <discord9@163.com> * fix(promql): report a raw chain that cannot pick a destination Two paths hid a destination they could not compute. A matching-routed chunk without labels built an error and then discarded it through `unwrap_or_else`, routing the chunk by its stored key instead; and `output_labels` answered the stored-key policy with nothing at all, so every chain would have gone to one partition silently. Neither is reachable today -- a raw frame is required to carry explicit labels, and a stored-key-routed frame never asks for a label hash -- but a silent fallback turns a defect into a wrong answer. Both now say what went wrong. Signed-off-by: discord9 <discord9@163.com> * test(promql): cover the histogram properties a duplicate check has to compare Merging two raw pieces treats a sample seen twice at one timestamp as a duplicate only when every property of the two matches. The comparison already covered all of them, but its test changed only the float ones, so dropping `reset_hint`, `start_timestamp` or `custom_values` from the comparison left every test passing: conflicting samples would have been discarded as duplicates with no failure. Each of the three now has a case that differs in exactly that property. Removing any of the three comparisons fails the test. The bucket side is deliberately not covered: a legal sample must satisfy `span_lengths` summing to the bucket count with an in-range cursor, so a sample differing in one bucket field is a different shape rather than a different value. Signed-off-by: discord9 <discord9@163.com> * perf(promql): publish an ordered identity as its inputs pass it, and charge what a merge uses A key-then-time identity could only be published after every input ended, so the node buffered a whole region even though the ordering it was protecting is exactly what lets it publish early. An input emits its keys in order and each chain goes to one owner, so the keys one owner sees from one input are non-decreasing too: a greater key from an input proves that input will not contribute a smaller one. The owner records the greatest key seen per input and publishes the pending identity with the smallest key once every input has contributed it, ended, or been seen past it. Readiness is checked in ascending key order, so the output stays ordered. An identity carried by a single piece needs no reconstruction at all: it is marked final and published as it is, which is what the evaluated path already did. A multi-piece identity charges each allocation the reconstruction actually makes — the two row vectors, the timestamps, and one descriptor per surviving run — instead of reserving a worst case per row per field that a well-ordered identity never approaches. The two tests for the ordered path were written against a fixture where the key they expected to be held was already complete, so they asserted the opposite of the rule. They now use fixtures where the held key really has not been passed, and they collect rows per identity because a frame may split one identity across chunks. Signed-off-by: discord9 <discord9@163.com> * fix(promql): drop the uncharged merge wrapper the charging path replaced Routing a raw identity charges each allocation before it is made, so `merge_raw_identity` had no caller left outside the tests and `cargo clippy -p promql --all-targets -- -D warnings` refused it as dead code. Its body is now the test helper that calls the charging form with a no-op reserve, and the production name is gone. Signed-off-by: discord9 <discord9@163.com> * fix(promql): stop scanning settled identities, and keep merge errors typed Three faults in the raw owner, each small: A key-then-time frame publishes only in key order, so if the smallest pending identity is not ready no larger one can be: an input reaches a later key only by passing the earlier one. The rescan applied that predicate to every pending group anyway, so an input that fell behind could have the owner walk the whole map after each arriving chain it could not yet settle. Ask the first entry only. `reserve` returning `ResourcesExhausted` came back through `invalid(...)`, so a memory-pool rejection reached the caller as a malformed-series error with the wrong status. A reservation failure is not a claim about the data; hand the `DataFusionError` through untouched. The merge reserved its row and selection scratch up front and kept holding both after `rows` and `selected` were dropped, since the reservation outlives them until the packer takes the output. Release the pair once the merge is done. The release amount matches the two reservations, so it can only fail if the reservation was already reduced elsewhere, which is why the assertion checks it but the query still succeeds. Signed-off-by: discord9 <discord9@163.com> * test(promql): assert the decode a chain-atomic merge produces This case asserted that unpacking fails with "evaluated series identity changed before its chain closed". It was written to keep that boundary honest until the region merge became chain-atomic, and its own comment asks for this change once that landed - which it has: unpacking now succeeds. The success it asserts is the same property read the other way. Sixteen hosts, one chain each, each closing exactly once, each reporting 1.0 - the rate its own samples produce, since the fixture's value rises one per second. A merge that started interleaving chains again would cut them, and a cut chain cannot report one correct value per host. The case's evaluation grid holds one step, so it says nothing about how a chain spanning several chunks behaves; that is a property of the grid, not of the merge, and the comment now says so instead of claiming four batches per chain. Signed-off-by: discord9 <discord9@163.com> * test(promql): cover the gaps the review named, and drop two false claims Four independent gaps in the raw repartition tests, plus one comment that asserted something the code does not do. `RawScript` reported `SeriesOrdering::Contiguous` regardless of what its plan declared. `pack_raw_series` derives both the frame metadata and the `SequenceValidator` from that one value, so every `KeyThenTime` test packed `Contiguous` batches and validated them under the `Contiguous` contract — `SequenceValidator`'s "raw series keys are not ordered" check never ran. Carry the declared ordering through `RawScripted` and hand it to the stream, so the batches and the check agree with the plan. `raw_matching_routing_groups_by_match_key` asserted only that both identities shared an output, which holds under primary-key routing too because those two keys collide in a three-output layout. Assert the owner against `output_labels` of the match key instead, so routing by the wrong key fails. The test named for waiting asserted early publication through the watermark. Rename it, and cover the other side deterministically with `gate_chunks`: an identity is withheld while another input has delivered nothing, then publishes once the gate opens. The two lifecycle tests set `gate_chunks: true`, which blocks the *first* chunk, so nothing was ever charged and their wait for a reservation could not be satisfied. Let the chunks through and keep the gate closed at EOF: the chain is delivered, charged, parked, and then dropped. `histogram_duplicates_compare_every_property` claimed the bucket side was unreachable because a legal sample must satisfy layout constraints. Only the bucket *count* is constrained; bucket values are free. Add a case per property — schema, count, zero_count, sum, zero_threshold, either side's bucket value, either side's span offset and span length with equal counts, plus the existing reset_hint, start_timestamp and custom_values — so dropping any comparison from `histograms_equal` fails. `native_prefix_multi_partition_series_chains` said the region merges chain-atomically. It does not: `CoalescePartitionsExec` still interleaves partitions batch by batch. The decode passes because #10023 closes a region's pending raw chunk at package completion, so a chain's value and its close are no longer latched apart. Say that, drop the unsupported "four batches per chain" count, and note that a one-point grid gives one value plus a possible empty close. Signed-off-by: discord9 <discord9@163.com> * perf(promql): merge ordered raw pieces without a row index `merge_raw_identity_charging` rebuilt a `(timestamp, chunk, row)` entry for every row of every piece and sorted the lot, then walked it again to drop duplicate timestamps. Reconstructing one series therefore cost `24 * rows` bytes of transient workspace and an `O(n log n)` sort, even when the pieces already arrived in order. The validation above that point has already refused a piece whose own timestamps are not strictly increasing, so a piece never needs sorting internally. What is left to order is only the relation between pieces. When every adjacent pair satisfies `previous.last() < next.first()` the concatenation is already the merged sequence, and no timestamp can repeat across pieces, so neither the index nor the duplicate walk is needed. Keep the general path unchanged for the interleaved and touching cases - touching pieces share a timestamp, so they still need the duplicate check - and shrink only what the path actually reserved. Signed-off-by: discord9 <discord9@163.com> * test(promql): vary histogram spans one dimension at a time `histogram_duplicates_compare_every_property` varied `span_offsets` and `span_lengths` together in the two length cases: the baseline was `offsets: [0], lengths: [2]` and the cases used `offsets: [0, 0], lengths: [1, 1]`. The offsets comparison rejected them first, so removing the lengths comparison left the test green and those two cases pinned nothing. The baseline is now two spans in both sides (`offsets: [0, 0]`, `lengths: [1, 1]`). The offset cases change only the cursors (`[1, 0]`) and the length cases only the lengths (`[2, 0]`), keeping the cursor list, the span count and the bucket count fixed, with a zero-length span as the only difference. Ablating any one of the four comparisons - positive and negative offsets, positive and negative lengths - now fails the case that covers it. `single_chunk_uses_ordered_fast_path` is removed. It passed with the fast path forced off, so it did not pin path selection, and the owner never reaches the merge with one chunk: `finish_raw_group` returns the chunk after setting `is_last`. The fast path stays pinned by `ordered_disjoint_pieces_concatenate_without_row_workspace`, which asserts the exact reservation and zero shrink for disjoint chunks, and by `touching_ordered_chunks_use_general_duplicate_path` on the other side. Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com>