LogQL: index the label vector inside the | json flatten (#447) - #448
Merged
Conversation
`insert_flattened` resolved every emitted key with `label_position`'s `labels.iter().position(..)`, so a row emitting m labels cost m(m-1)/2 string comparisons. One uninstrumented `run_into` over a 4 126 651 B flat object (m = 67 650) cost 34-42 s before this and 0.11-0.23 s after — the `LADDER-WALL ... B=4126651` line of `CARGO_INCREMENTAL=0 cargo test -p pulsus-read --test logql_row_expansion_ladder -- --include-ignored --nocapture --test-threads=1`, 4 runs before and 60 after, one process per run. Both figures are RANGES because the measuring machine is shared: several builds run on it concurrently, the 4 MiB rung's spread is about 2x, and no single value inside either range is reproducible on demand. The gap between the ranges is the finding — every point in one is two orders of magnitude from every point in the other — and the file's docs now say so at each place a wall time appears. Adds `LabelIndex`: an open-addressed `Vec<u32>` holding positions, not names, so every probe compares against `labels[i].0` — the same comparison `label_position` makes, which is why the answers coincide by construction — and the table allocates nothing per key. Built once per flatten and dropped with it, because positions are stable only while the vector is append-or-overwrite-in-place, which is exactly that walk's shape. First occurrence wins, matching `position`. Behaviour is unchanged; the differential test pins the equivalence over duplicates, `_extracted` collisions, empty and non-ASCII names, and the load-factor test pins the invariant that makes the probe loop terminate. Two knock-on records in `tests/logql_row_expansion_ladder.rs`: `F` moves 3.583 -> 3.934 (worst case 240 451 059 B -> 264 006 270 B) on the index table's doubling transient, with the pre-committed rule still selecting the same branch; and `zz_the_full_precommitted_ladder` loses its `#[ignore]`, which the file itself records as wall-time-only, because the test now costs 1.2-2.0 s. Those two byte figures are the only numbers in that file still written as single values, and they earn it by being gated: `apply_the_rule` now asserts them by EQUALITY against new `F_MILLI` and `WORST_CASE_BYTES` constants, alongside the pre-existing bounds and branch-selection assertions, which pass over a wide band and would not have caught a stale figure. They are deterministic allocation counts under a pinned toolchain, so an exact gate is available for them in a way it is not for anything timed. The prose repeating those two figures is gated too, by a count rather than by a list. `the_quoted_figures_are_counted_so_a_new_quotation_cannot _go_unchecked` reads the file's own source through `include_str!` and counts occurrences of each figure's rendering, against `F_QUOTATIONS` and `WORST_CASE_QUOTATIONS`; the needles are built from the two constants at run time, so neither literal appears in the searcher and the count is of quotations elsewhere. A hand-written inventory of sites was tried first and was wrong on the day it was written — it has no failure mode, and the edit that adds a mention silently falsifies it. Issue #241's closeout comment is the one quotation outside any test's reach; it stays named in prose because nothing in the tree can check it. Claude-Session: https://claude.ai/code/session_01B1xu1frZL9oF8MYRhkiMhv
`assert_gzip_identity_metrics` scraped `/metrics` twice and required the two decoded bodies to be equal after filtering out exactly one metric name. `ops::metrics_handler` bridges live counters and gauges into every scrape, so any second value that starts moving fails it; it flaked on PR #446 on a change touching no metrics code. The race was only ever in the body comparison, so only that changes: the body equality now comes from ONE response gzipped locally. The filter is deleted rather than left dead. Both legs keep their own header cells, each asserted against its own response and never against the other's — the identity leg's Content-Encoding ABSENCE is a per-route cell, and the matrix exists to catch a route that stops going through the global CompressionLayer, which the other routes' passing legs cannot show. Headers of two scrapes do not race. Claude-Session: https://claude.ai/code/session_01B1xu1frZL9oF8MYRhkiMhv
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #447.
What was slow
A LogQL query using
| jsonon a stream with many distinct labels re-scanned the label vector for every extracted field. The cost grew with the product of the two, so a wide stream with a wide| jsonspent most of its time re-walking a list it had already walked.What changed
Commit 1 indexes the label vector once inside the flatten, so lookups are a hash probe rather than a scan. Commit 2 is unrelated to the index: the
/metricsgzip conformance test was comparing two separate scrapes, which cannot be equal because counters move between them.The measurement, and how to read it
At the 4 MB rung of the row-expansion ladder, one process per run:
These are ranges from a shared build machine, not single figures. Wall-clock timings here carry contention from whatever else is building — during this work the 15-minute load average reached 33, and a run at load 5.58 gave 163.7 ms, inside the range. Pinning to one logical CPU made it worse (median 161 ms) while running at the lowest load of three batches, so load does not predict this figure at this scale. Every timing figure in the branch is published as a range with its run count and that condition; the superseded single figures are kept in place as the record.
Two figures are deliberately not ranges:
F = 3.934and264 006 270 B. They are deterministic byte ratios, not wall times, and they are now pinned by equality assertions (F_MILLI,WORST_CASE_BYTES) rather than by the broad bounds that previously surrounded them — an earlier round found the docs described them as assertion-checked when nothing checked them.Enumeration as a count, not a list
The doc that told a future re-derivation where these figures are quoted was a hand-written list, and it was wrong — it named three sites when there are nine occurrences of one figure and three of the other. A list of occurrences has no failure mode: the next edit that adds a mention silently falsifies it.
It is now a test that reads its own source and counts, asserting against named constants. Search needles are built at run time from the constants themselves, so the needle cannot drift from the value it tracks and the searcher cannot find itself. Breaks were observed in both directions — adding a mention (10 vs 9, 4 vs 3) and removing one (8 vs 9) — on the shipping source.
Review trail
Four rounds on the code, ending
## Review — code (round 4) — VERDICT: PASS on be94743. Rounds 2 and 3 each produced findings that were fixed; two findings in round 2 were withdrawn by ruling because the review brief was over-specified, recorded on the issue with the reasoning.Workspace suite reconciles at 6430 passed / 25 skipped across fourteen crates. Both commits revert independently on disjoint file sets.
https://claude.ai/code/session_01B1xu1frZL9oF8MYRhkiMhv