mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-11-14 06:46:56 +00:00
discord9
eeca48bc43
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) - Spec: Narthex Unified SeriesFrame refactor, PR A of two. - #9914, #9868, #9937, #9896, #9933, #9936, #9882 and #9886 are all merged, so this is a plain increment on `narthex` with no stacked dependency. ## What's changed and what's your intention? SeriesFrame is the internal transport of the Narthex native PromQL path, and it could only carry raw samples: an evaluated result had to fall back to a plain row table. This makes **one self-describing Arrow format carry both**, so a receiver decodes a frame from the frame alone, without the scanner, region, or planning context its producer had. What is unified is the **physical representation and its codec**, not the runtime data structures. `RawSeriesChunk`, `Series` and `EvaluatedChunk` are unchanged; only the adapter boundaries branch on the sample shape. ### Base - base: `narthex` @ `771347df5d9` - HEAD: `8bc53c48714` - **no stacked dependency**: everything this PR built on has landed This is a plain increment on the current `narthex` tip, with focused follow-up fixes for review and CI. Nothing from the earlier stack is carried: #9914 is merged, so no `MetricName` rename appears here. The codec itself, the adapters, the tests and the two frozen wire fixtures are all that moved, plus the call sites that had to follow the renamed API. ### The format (`layout = v2`) One frame family, three legal combinations. Every other combination is refused on both the writing and the reading side: | content | identity | time | producer | | --- | --- | --- | --- | | `raw` | `key` | `timestamps` | a storage raw-series scan | | `raw` | `explicit` | `timestamps` | a raw input whose labels are already decoded | | `evaluated` | `explicit` | `grid_spans` | a compute node's output | `evaluated` + `key` is refused because a computation may rewrite its labels, so it cannot claim a storage key as its identity. `raw` + `grid_spans` and `evaluated` + `timestamps` are refused because raw samples have no grid and computed values have no timestamps of their own. Column layout (`series_frame_schema` in `src/datatypes/src/series/frame.rs`): ```text read_key: Binary (NOT NULL for key mode; NULLABLE for explicit mode) is_last: Boolean NOT NULL time: Struct<timestamps: List<Int64>> // raw Struct<spans: List<Struct<start: u64, len: u64>>> // evaluated fields: Struct<<field_name>: SamplePayload, ...> name: Utf8 NULLABLE // explicit only, __name__ labels: List<Struct<name, value>> // explicit only, no __name__, no duplicates ``` `read_key` must be populated for every raw row, including raw/explicit, and must be NULL for evaluated rows. `SamplePayload` (float / native histogram / mixed) keeps its existing representation and is not redesigned. Protocol metadata lives under `greptime.series.*`. `SeriesFrameDeclaration::parse` reads the layout, identity, time and ordering declaration. Schemas explicitly declare `contiguous` by default; raw streams may declare `read_key_then_time`, while evaluated streams accept only `contiguous`. Missing ordering, unknown values and unknown reserved keys are refused by declaration parsing, direct decoding (including empty batches), and stream admission. | key | value | | --- | --- | | `layout` | `v2` | | `content` | `raw` / `evaluated` | | `labels` | `key` / `explicit` | | `time` | `timestamps` / `grid_spans` | | `grid` | required for evaluated, forbidden for raw; canonical `GridSpec` JSON | | `label_decode` | required for key, forbidden for explicit | | `ordering` | `contiguous` / `read_key_then_time` | ### Time axis - raw: every row carries its own strictly increasing `i64` millisecond timestamps. - evaluated: the `GridSpec` is declared once in schema metadata (`{start_ms, step_ms, len}`), and a row carries only the **absolute grid positions** as spans. A sparse grid is never expanded and its holes are never filled; `start + len` is checked against the grid with checked arithmetic. - A zero-sample closing chunk is legal for evaluated and **is kept** as one frame row with zero positions and `is_last = true`, because dropping it would drop the finality it exists to carry. A zero-sample chunk that closes nothing is refused, and a raw frame still refuses an empty chunk. ### Series chains and `is_last` `is_last` closes **one chunk chain**, not the end of a PromQL label set. After a native label rewrite, two different input series can produce **the same complete labels**, and each closes itself. The writer allows that and the reader does not treat "same labels" as a re-opened series. A chain that changes identity before closing, and EOF with an open chain, are both errors. ### Key code locations - `src/datatypes/src/series/frame.rs`: shared schema, declaration parser, bounded builder, cursor and raw/evaluated decoder. - `src/common/query/src/series_frame.rs`: typed pack/unpack adapters and chain validators. - `src/datatypes/src/series/time.rs`: canonical `GridSpec` serialization. ### Review and CI fixes - Repaired the two table PK-binding fixtures with real `host` and `rack` Tag columns and matching primary-key metadata. Production `LabelDecodeSpec::validate()` stays strict. The mismatch test retains all six cases, including changed tag order, and checks rejection before polling. - Removed the missing-ordering fallback and added one private ordering validator shared by the declaration parser and direct decoder. Invalid evaluated ordering cannot bypass stream checks. - Compatibility metadata is tested exactly as written, without merging canonical metadata into malformed inputs. The direct-decoder matrix covers raw/evaluated, empty/nonempty, missing, unknown and interleaved ordering, and valid raw versus invalid evaluated `read_key_then_time`. - Independent source and anti-over-design review: **APPROVE** for `771347df5d9..8bc53c48714`. Review approval is separate from the CI status of the new head. ### Limitations - Not reachable from a query yet: `FusedSeriesExec` still answers `PlainRows` by default. The second output format is PR B. - Not verified: Windows; the results below are Linux. - Deliberately **not** done, per the spec: evaluated **input** wiring in `FusedSeriesExec`, Complete Aggregate's SeriesFrame input, Partial/Final physical plans, cross-region exchange, remote NativeTree serialization, NodePath rework, #9892 storage-interface changes, #9844 remote read wiring. - The unreleased v1 frame format is **not** read. In-tree producers, consumers, tests and the two frozen wire fixtures all move to v2 together. ### Does this break API or data compatibility? No published format changes. SST, WAL, manifest and the generic Flight/Protobuf formats are untouched. The only versioned thing here is the unreleased internal SeriesFrame layout, which goes from `v1` to `v2` in one step and is not read in both directions. ### Tests The existing tests include real pack → batch stream → unpack round trips, not only schema snapshots: - raw/key with its decoded PK, raw/explicit with complete labels, both across batches and through Arrow IPC - evaluated dense, sparse (multiple spans, holes not filled), multi-field (a position can hold a value in one field and nothing in another; NULL ≠ NaN), native histogram and mixed payloads (payload kind and values survive, no silent float-ification) - evaluated empty closing chunk kept as one row; multi-batch streams keeping order and finality; two consecutive chains with equal labels each closing independently; EOF with an open chain - malformed frames refused: non-increasing timestamps, overlapping/out-of-order/out-of-bounds/zero length spans, payload length mismatch, span-count overflow, missing grid, malformed `GridSpec`, missing label decode, contradictory metadata, wrong layout version, an evaluated row carrying a storage key, an empty row that closes nothing <!-- CURRENT_VERIFICATION_START --> Verified on Linux at `8bc53c48714`; every command below exited 0: ```text make fmt-check -> clean (rustfmt and repository source checks) cargo nextest run -p datatypes -p common-query -p table --retries 0 -> 690 passed, 0 skipped; includes both reported CI failures cargo nextest run -p mito-codec -p promql -p query --retries 0 -> 2055 passed, 2 skipped cargo nextest run -p mito2 -E 'test(engine::scan_test::test_mito_handle_query_raw_series) | test(engine::scan_test::test_two_phase_native_raw_stream) | test(engine::scan_test::test_two_phase_raw_) | test(engine::scan_test::test_mito_raw_series_)' --retries 0 -> 8 passed, 1733 skipped (focused raw-scan selection) cargo check --workspace --all-targets -> passed cargo clippy -p datatypes -p common-query -p table -p mito-codec -p promql -p query --all-targets -- -D warnings -> passed (scoped crates, default features) ``` License headers, TOML formatting and `git diff --check` also pass. These are local Linux results, not a claim that the new-head GitHub checks are green. Full `make test`, workspace all-features Clippy and Windows execution were not run locally in this round. <!-- CURRENT_VERIFICATION_END --> ## PR Checklist Please convert it to a draft if some of the following conditions are not met. - [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`).