mirror of
https://github.com/GreptimeTeam/greptimedb.git
synced 2026-11-14 06:46:56 +00:00
discord9
115d85b669
* fix(mito2): identify series by their exact sparse key, not by tsid `(table_id, tsid)` is a hash, so distinct series can share it: 2000 label values already produce 20 colliding pairs and a 36k-series SST keeps only 27856 distinct tsids. The columnar series writer silently merged such pairs (first label set kept, row counts added) and the samples writer either reported time_regressed or, with increasing timestamps, attached one series' samples to another's labels. Every one of those decisions used the hash as identity. The SST already stores the exact sparse primary key, so nothing on the storage side changes: `tsid`, the hash function and the key layout are untouched. What changes is that the artifacts, their readers and the prepared read path now decide identity on the canonical sparse key bytes, and `(table_id, tsid)` is only the coarse, order-preserving prefix used for ordering, pruning and bucketing. - `ExactSeriesKey` is the new identity type: equality, ordering and hashing are defined on the key bytes alone, `id()` is documented as a derived prefix that is never compared. `from_sparse_key` validates the entire key, not just the prefix, and bounds it at 8 KiB. - Series format v2 keeps one row per exact key; the descriptor layout and the existing per-row tags already express this once the writer aggregates by key. Coarse page/block/part bounds may now touch, so readers prune conservatively. - Samples format v2 records the owning key per block, appends a `__primary_key` column, and lets a key change restart timestamps and ordinals while the same key keeps the monotonic rules. Exact selection travels in `SamplesReadRequest.keys` next to the coarse `ids`. - The prepared path keys candidates by `ExactSeriesKey`, re-encodes identity from the artifact's own tags, associates samples through `__primary_key`, and uses the exact key for the merge key and the direct-label path. - Both manifests carry a mandatory `identity_encoding` discriminator. Artifacts without it are structurally valid and perfectly ordered yet may contain an already merged row, so they are rejected and rebuilt from SSTs; maintenance rotates them out instead of reusing them, and the local cache version drops the rebuildable store. There is no dual reader and no migration. Verified: `cargo nextest run -p mito2` 1824 passed, 2 skipped (three times, identical); `cargo nextest run -p mito-codec` 57 passed; clippy `-D warnings` and rustfmt clean. Tests pin the real captured pair (`host-020078` / `host-029838`, both retained with their own labels), the silent-loss variant with globally increasing timestamps, sixteen distinct keys under one forced tsid in both PK encodings, candidate isolation end to end, and rejection of a frozen v1 fixture. Signed-off-by: discord9 <discord9@163.com> * fix(mito2): close the exact-identity review findings - The two-phase series scan assumed candidate discovery had already enforced tag predicates through an "exact assigned-series set". An assignment is a `(table_id, tsid)` hash, so a colliding partner shares it without matching the predicate, and skipping tag filters returned its rows. Apply the predicates the precise filter already holds instead of assuming them. - `MAX_KEY_BYTES` was checked when reconstructing a key from an artifact but not when a writer built one, so the writer could publish a key the reader refused; a single 8 KiB label already encodes past the cap. Length is bounded by the containers that store a key (one key per block, descriptor, page and column limits), and long labels are real data, so drop the cap instead of enforcing it on both sides. - The `__primary_key` column copies one key per output row, but the read reservation budgeted a fixed 32 bytes per column and metadata pages did not count the per-block key heap: 8192 rows of 8 KiB keys allocated ~64 MiB against a ~1 MiB estimate. Budget the real bytes and count the heap. - `SamplesReadRequest` carried both `keys` and `ids`, which the only caller derived from the same candidates; selection is key-only now and the reader derives its coarse pruning set, so the two selectors cannot disagree. - Restore the unknown-field-codec projection coverage that the frozen v1 fixture used to carry, against a generated v2 artifact, and assert v1 rejection through the compatibility property rather than a field name. - Drop the redundant columnar increasing-timestamp test (that writer never reads timestamps) and correct the samples format doc: the five internal columns and `__primary_key` are non-null, value fields stay nullable. Verified: `cargo nextest run -p mito2` 1830 passed, 2 skipped; clippy `-D warnings` and rustfmt clean. Each new test was run with its fix reverted first: the two-phase test returned the colliding partner's rows, the long-key tests failed with "series key exceeds 8192 bytes", and the 4 MiB pool test read 8192 rows of 1 KiB keys while allocating ~8 MiB. Signed-off-by: discord9 <discord9@163.com> * fix(mito2): budget the exact key column exactly and drop stale skip-tag claims The read reservation charged the exact-key column its logical bytes, but the column was built with `BinaryArray::from_iter_values`, whose value buffer grows geometrically: measured on a 2135-row fixture it retained 4,989,432 bytes for 2,551,325 bytes of key data (1.956x), so a pool sized from the reservation was oversubscribed. `BinaryArray::new_repeated` allocates the offsets once and reserves the key bytes once (1.003x on the same fixture), which is what the reservation already assumes. The new test asserts the pool covers what the yielded batch retains, that the column keeps no growth slack, and that the reservation is released on drop; 2135 rows sits just past a doubling step so a growing builder fails it. Also make the documentation match the mechanism and the encoder: - `read/pruner.rs` still told readers the data phase skips tag filtering because candidate discovery had applied the predicates exactly once. Assignments are `(table_id, tsid)` hashes, so a colliding partner shares the identifier without matching the predicate, and the data phase does apply tag predicates. - The initial-window fallback test anchored on `reads[5]`, which is the second payload window; it now identifies the initial window by its payload-header offset and asserts the window spans 128 blocks, so a traversal change cannot quietly point it at another read. - The samples format descriptor sizes are recomputed from the encoder (`48 + 44 * C + key_length`): `224`, `45,236`, and with the leaf prefix `45,248`, each `+ key_length`; the page-fit sentence now states the enforced constraint (descriptor and leaf prefix inside one page) instead of claiming the widest descriptor always fits. Verified: `cargo nextest run -p mito2` 1831 passed, 2 skipped; clippy `-D warnings` and rustfmt clean. Signed-off-by: discord9 <discord9@163.com> * fix(mito2): budget each projected sample array's fixed overhead The read reservation charged per-row bytes plus the payload sizes, but not the fixed cost of the arrays it builds: Arrow's `get_array_memory_size` is the array struct plus its buffer capacities, so a one-row, key-only projection reserved `key_length + 72` for the 1344 bytes it actually retained. Add `ARRAY_HEADER_BYTES` (`size_of::<BinaryArray>()`) plus `BUFFER_ALIGNMENT` per projected column, as their own term, and cover the one-row key-only case in `exact_key_output_reservation_covers_the_retained_array`: the pool must cover the batch's reported memory size while it is alive and be empty after drop. Verified: `cargo nextest run -p mito2` 1831 passed, 2 skipped; clippy `-D warnings` and rustfmt clean. Signed-off-by: discord9 <discord9@163.com> * chore(mito2): pin the series-index cache token to 1 The branch inherits `2` from an earlier layout change. This branch is experimental and has no deployments, so it pins the token at `1`: whatever a different build wrote into the rebuildable local store is discarded on startup and rebuilt from the SSTs. The token is not the compatibility boundary on its own. Artifacts are rejected on open when they lack the mandatory `identity_encoding` discriminator, so a hash-only artifact is never served even when the reset has not run yet (a read that precedes it, or an artifact outside this cache). Signed-off-by: discord9 <discord9@163.com> * feat(mito2): keep exact series keys out of the samples descriptor page A block descriptor used to embed the whole exact key, and a descriptor shares a 65,536-byte metadata page with its leaf prefix. That put an accidental ceiling on key length: 65,344 bytes with no value fields, 65,300 with one, and 20,288 with 1,024 fields. Valid SST primary keys can be longer, so their samples builds failed with "sample descriptor exceeds leaf limit" even though the key itself was accepted. The key now lives in a per-part key area and the descriptor carries a reference: part = [header 16][column payloads][key area][leaf pages / root][trailer 120] trailer = root ref 72 | depth 4 | version 4 | file_length 8 | payload_end 8 | key_end 8 (body 104), magic 8, XXH64 over the preceding 112 bytes 8 block = table_id u32 | tsid u64 | key offset u64 | key length u32 | min i64 | max i64 | ordinal u64 | rows u32 | columns u32 (56 bytes, fixed) Descriptor size no longer depends on key length (232 bytes for one present field, 45,244 for 1,024), so the ceilings are gone. Keys are written once per series, back to back in block order and without padding, so an equal reference means "same exact series" and a key may exceed any page limit; a reference is validated against `key_end` with a checked add, an overlap between consecutive new series is rejected, and a resolved key whose `(table_id, tsid)` disagrees with its descriptor is refused before any row is emitted. Reads stay lazy: only blocks that survive coarse selection fetch a key, at most once per series, merged per page and bounded by `MAX_READ_BYTES`, reused from the part's aligned cache when one is configured. Measured on the 260-series fixture: 260 per-series key reads before, 1 merged read after, with the fetched byte count asserted to be exactly the key bytes. The read contract also stops materializing one key per row: `SamplesBatch { batch, key }` carries the block's key once, the exact-key column is refused in projections, and `prepared` attributes a whole batch with a single binary search. For an 8,192-row block with a 1 KiB key the reservation drops from 8,388,608 B to 1,120 B (~7,490x), which is what lets a 4 MiB pool read it; `Candidate` keeps one owned copy of the key instead of two, and `retained_rows` is gone in favour of the `prepare_retained_rows` metric. No version bump: the layout changes in place. Artifacts written by an earlier shape of this branch (112-byte trailer, or keys inside descriptors) are unreadable and must be rebuilt, which the cache token and the required `identity_encoding` discriminator already bring about. One user-visible cost: keys retained by a scan are charged to its memory pool, so a scan that used to fit in 16 KiB needs ~14 KB more for 128 series. The fixture that pinned 16 KiB is raised to 64 KiB with the arithmetic in a comment. Verified: `cargo test -p mito2 --lib -- series_index::samples series_scan::prepared series_index::identity` 72 passed / 0 failed / 2 ignored on six consecutive runs; clippy `-D warnings` and rustfmt clean. Failing-before evidence, both produced by reverting the relevant hunk: - restoring `48 + 44 * C + key_len` fails the >64 KiB round-trip with "sample descriptor exceeds leaf limit"; - the 260-series coalescing assertion fails against per-series reads. Signed-off-by: discord9 <discord9@163.com> * test(mito2): assert the duplicate-drop on the preparation's own rows `overlapping_buckets_retain_only_new_label_rows` compared the deltas of two process-global counters (`prepare_unique_series`, `prepare_retained_rows`). Every preparation in the process increments both, so under parallel test execution the two deltas do not pair up and the test failed intermittently - it always passed in isolation. Assert the property the pairing was standing in for, on this preparation's own rows: read every series of the prepared source and require that no series keeps two rows at the same timestamp. A surviving duplicate from the overlapping bucket would repeat a timestamp; the counters can no longer make the result depend on what else the test binary happens to be running. Signed-off-by: discord9 <discord9@163.com> * test(mito2): drop the now-unused preparation counter helper The duplicate-drop assertion no longer reads the process-global counters, so `prepared_count` has no callers and `-D warnings` rejects the dead function in `cargo check` and `cargo clippy` CI. Remove it. Signed-off-by: discord9 <discord9@163.com> * fix(mito2): scope key references per part, bound key reads, checksum keys Review of the key-area change found three defects that ordinary data can hit and three smaller ones. All six are fixed with a test that fails beforehand. Key references are file-local offsets, and the ordering state kept them across parts: a series continuing into another part looked like an overlapping reference (or a new series with a non-zero ordinal), and two different series in structurally identical parts could be mistaken for one because their offsets and lengths coincided. Reference ordering is now scoped to a part; continuation across a boundary is decided on the exact key bytes plus time/ordinal, and the loaded keys must be nondecreasing, a repeated key must continue its series. A key prefetch fetched the whole window and only then grew the reservation, and the per-key fallback charged after its read, so a small pool could read and assemble a page of long keys before reporting exhaustion. The peak (fetched entry + owned key + alignment/assembly) is now reserved before the read; on refusal the reader drops alignment first, then halves the window, and a returned key is cloned only after the block reserved for it. A test asserts that an unpayable prefetch issues no key read at all. The key area had no integrity check, so flipping one byte of a label - leaving framing and `(table_id, tsid)` intact - could silently change which series a block belongs to. Every stored key now carries its XXH64, excluded from the reference length and verified when fetched. The manifest discriminator now names the layout (`exact-sparse-key-area` for samples; the series index keeps `exact-sparse-key`), so an artifact with inline keys is refused at open instead of being marked reusable by maintenance and failing later at a part read. No version number changes anywhere. A key copy is also counted correctly when a series spans a metadata rollover: the "first copy of this part" decision is taken after the rollover, not before. Verified: `series_index::samples` 51 passed, `series_scan::prepared` 27 passed, `series_index::identity` 4 passed; clippy `-D warnings` and rustfmt clean. Signed-off-by: discord9 <discord9@163.com> * fix(mito2): charge the key peak on every path and chain the order check Four follow-ups from the confirmation review of the key-area change. The per-key fallback reserved only the retained and stored bytes and then fetched through the aligned payload cache, whose aligned/assembly peak it never charged, so a leftover key could read a whole aligned block the pool had not paid for. It now reserves the same peak as the page prefetch: aligned budget first, and on refusal an explicitly cache-less reader with the unaligned peak, before any read. The key carried across a part boundary was a deep copy taken while the per-part reservation was being released, so it lived on uncharged until the next part. It is now held by its own pool consumer, charged before the copy and released when it is replaced or when the boundary falls behind the scan. The content-order check was chained over the last *traversed* block, which an unloaded block overwrote, so a block pruned by selection could hide reversed or restarted keys - and could drop the boundary key entirely when a part ended on a pruned block. It is now chained over the last *loaded* key, with adjacency taken from "the last traversed block is the last loaded one", so a gap still requires increasing order and a strictly greater ordinal but not exactly +1. Finally, the key-area read counter in the tests only saw reads fully inside the key area, missing aligned reads that start before it; it now counts reads that intersect, and the cached-read test is documented as a cache-behaviour guard rather than a failing-before proof for the budget work. Verified: `series_index::samples` 55 passed, `series_scan::prepared` 27 passed, `series_index::identity` 4 passed; clippy `-D warnings` and rustfmt clean. Failing-before evidence: a 16384-byte key-area read issued with only 15080 reserved; a boundary key with 0 of its 1195 bytes charged; reversed keys hidden behind a skipped block; and a pruned block hiding the boundary key. Signed-off-by: discord9 <discord9@163.com> * docs(mito2): qualify the ordinal rule to one exact key The samples format said a loaded key's ordinal must still grow, which reads as if ordinals had to increase across different series too. They do not: only an equal exact key continues, an adjacent block of that key requires +1, and a different key starts at zero. Signed-off-by: discord9 <discord9@163.com> --------- Signed-off-by: discord9 <discord9@163.com>