chore(bench): sort-key / read_in_order spike scaffolding (ClickStack Phase 3)#171
chore(bench): sort-key / read_in_order spike scaffolding (ClickStack Phase 3)#171Makisuo wants to merge 1 commit into
Conversation
|
Your LLM provider API key was rejected. Rotate the key in your provider dashboard, then update the matching GitHub Actions secret. Update repo secret → · Model settings → · Setup docs → · Ask in Discord →
|
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| label: "control — current key, freshly rebuilt (compare like-aged, not vs the aged prod table)", | ||
| partitionBy: "toDate(Timestamp)", | ||
| orderBy: "(OrgId, ServiceName, SpanName, toDateTime(Timestamp))", | ||
| tsColumn: "Timestamp", |
There was a problem hiding this comment.
🔴 Trace sort-key probe queries the wrong column, so the benchmark will always report the optimization as not working
The read-in-order probe orders by the raw nanosecond-precision column (ORDER BY Timestamp DESC at apps/api/scripts/spike/sortkey-variants.ts:144) instead of the second-precision expression (toDateTime(Timestamp)) that appears in every traces sort key, so ClickHouse cannot match the query ordering to the sort key and optimize_read_in_order will never engage for any traces variant.
Impact: The spike will falsely conclude that the candidate sort keys do not activate read-in-order for traces, producing misleading benchmark results and potentially discarding a valid optimization.
Sort-key expression mismatch between probe and ORDER BY
All three traces variants (traces_t1, traces_t2, traces_t3) define their sort key using toDateTime(Timestamp) — e.g. apps/api/scripts/spike/sortkey-variants.ts:48 has ORDER BY (OrgId, toStartOfFiveMinutes(toDateTime(Timestamp)), ServiceName, toDateTime(Timestamp)). The Timestamp column in the traces table is DateTime64(9) (nanosecond precision), confirmed at packages/domain/src/generated/clickhouse-schema.ts:36.
readInOrderProbeSQL at apps/api/scripts/spike/sortkey-variants.ts:138-146 generates ORDER BY ${v.tsColumn} DESC, which resolves to ORDER BY Timestamp DESC for traces. Since Timestamp is DateTime64(9) and the sort key uses toDateTime(Timestamp) (second precision), these are different orderings — multiple nanosecond-distinct values collapse to the same second. ClickHouse cannot prove monotonic compatibility, so it will not engage read_in_order.
The logs variants are unaffected because tsColumn is TimestampTime (already DateTime), which matches the sort key expression directly.
Fix: either change tsColumn for traces variants to toDateTime(Timestamp), or add a dedicated probeOrderByExpr field to SortKeyVariant that carries the exact expression from the sort key.
Prompt for agents
The readInOrderProbeSQL function at sortkey-variants.ts:138-146 generates ORDER BY using v.tsColumn, which for traces is the raw DateTime64(9) column 'Timestamp'. But all traces sort keys use toDateTime(Timestamp) (second-precision cast). ClickHouse's optimize_read_in_order requires the query ORDER BY to match the sort key expression exactly, so the probe will always show a false negative for traces.
The fix needs to ensure the probe's ORDER BY expression matches what's in the sort key. Options:
1. Add a new field to SortKeyVariant (e.g. probeOrderByExpr) that carries the exact timestamp expression from the sort key (toDateTime(Timestamp) for traces, TimestampTime for logs), and use it in readInOrderProbeSQL.
2. Change tsColumn for traces variants from 'Timestamp' to 'toDateTime(Timestamp)'. This would also affect backfillSQL where it's used in WHERE clauses, but toDateTime(Timestamp) >= toDateTime('...') still works correctly for filtering.
Option 1 is cleaner because it separates the backfill windowing column from the probe ordering expression. The test at sortkey-variants.test.ts:54 that checks for 'ORDER BY Timestamp DESC' would also need updating to expect 'ORDER BY toDateTime(Timestamp) DESC'.
Was this helpful? React with 👍 or 👎 to provide feedback.
Measurement tooling to decide whether a time-bucketed primary key can replace the two-stage list-query cutoff (read_in_order). Ships NO production change. - spike/sortkey-variants.ts: candidate sort keys (t1/t2/t3, l1/l2/l3, OrgId always first) + shadow-table / backfill / read_in_order-probe SQL builders. - spike/emit-spike-sql.ts: dependency-free emitter (setup | backfill | probe | drop) — runs with bare bun in a fresh worktree. - spike/SPIKE-SORTKEY.md: runbook + explicit promote/reject decision criteria. Drives the A/B via `bench suite --table-map` against a dedicated ClickHouse. Tests: 10 spike + 11 bench. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Makisuo
force-pushed
the
feat/clickstack-sortkey-spike
branch
from
efc5a48 to
8909dd6
Compare
| export function readInOrderProbeSQL(v: SortKeyVariant, opts: { orgId: string; limit?: number }): string { | ||
| const limit = opts.limit ?? 50 | ||
| return ( | ||
| `EXPLAIN actions = 1, indexes = 1\n` + | ||
| `SELECT * FROM ${v.name}\n` + | ||
| `WHERE OrgId = ${lit(opts.orgId)}\n` + | ||
| `ORDER BY ${v.tsColumn} DESC\n` + | ||
| `LIMIT ${limit}` | ||
| ) | ||
| } |
There was a problem hiding this comment.
🚩 Probe query uses raw Timestamp (DateTime64) but sort keys use toDateTime(Timestamp) — read_in_order may not engage for traces
The readInOrderProbeSQL function emits ORDER BY Timestamp DESC (sortkey-variants.ts:144), but all three traces sort-key candidates use toDateTime(Timestamp) (a DateTime, seconds-precision) in their ORDER BY — e.g. traces_t3 has (OrgId, toDateTime(Timestamp)) at sortkey-variants.ts:56. ClickHouse's optimize_read_in_order requires the query's ORDER BY to match a suffix of the sort key. Whether CH recognizes Timestamp (DateTime64) as monotonically equivalent to toDateTime(Timestamp) (DateTime) is version-dependent.
This is intentional — the probe tests the real production query pattern (see packages/query-engine/src/ch/queries/traces.ts:512 which confirms production queries use ORDER BY Timestamp DESC). But if CH doesn't recognize the monotonic relationship, the probe will show no read_in_order for ANY traces variant (including t2/t3), making the spike inconclusive. The SPIKE-SORTKEY.md procedure (line 38) expects to see read_in_order on t2/t3 but not t1 — if it's absent on all three, the operator should check whether using ORDER BY toDateTime(Timestamp) DESC in the probe (or Timestamp directly in the sort key) changes the result before concluding the sort key doesn't help.
Logs variants don't have this issue: TimestampTime is already a plain DateTime and appears directly in the sort key.
Was this helpful? React with 👍 or 👎 to provide feedback.
1 participant
Stacked on #170 (uses the
bench suite --table-mapA/B harness from that PR).What this is
Measurement scaffolding, not a production change. It builds throwaway shadow tables with candidate sort keys, backfills one busy org's window, and A/Bs them with the committed benchmark suite — to answer: can a time-bucketed primary key activate
optimize_read_in_orderand let us delete the two-stage list-query cutoff, without regressing service-scoped reads, attribute-bloom pruning, or compression?Files (
apps/api/scripts/spike/)sortkey-variants.ts— candidate sort keys (t1/t2/t3,l1/l2/l3; OrgId always first for tenant scoping) + pure builders forCREATE TABLE ASshadow tables, per-day windowed backfillINSERTs, andEXPLAINread_in_order probes.emit-spike-sql.ts— dependency-free emitter (--mode setup|backfill|probe|drop), runs with barebunin a fresh worktree; pipe its output at a dedicated ClickHouse.SPIKE-SORTKEY.md— the runbook + explicit promote/reject decision criteria (list p50 −40% or read_rows −80%, service-scoped regression ≤ +25%, attr-bloom regression ≤ +25% read_rows, compression ≤ +15%).Candidate keys
*_t1/l1*_t2/l2(OrgId, toStartOfFiveMinutes(ts), ServiceName, ts)*_t3/l3(OrgId, ts)Testing
10 spike unit tests (SQL-builder shape, OrgId-first invariant, day-windowing, emitter modes) + the 11 bench tests, all green; apps/api typecheck clean.
Running it needs a dedicated ClickHouse
Per the runbook — not a Tinybird branch (shared prod compute → noisy wall-time; lean on deterministic
read_rows/read_bytes). Couldn't execute the live A/B in this environment.🤖 Generated with Claude Code