fix(clickhouse): key event projections tenant-first on an hourly bucket

[lohanidamodar]

Problem

addProjection() declared every per-dim event projection as:

SELECT metric, time, tenant, <dim>, sum(value) AS value
GROUP BY metric, time, tenant, <dim>

Two independent defects:

1. Tenant is third. A projection is sorted by its own GROUP BY order, so this discards the tenant-first granule skipping the base table is deliberately ordered for (ORDER BY (tenant, metric, time, id)). Every read filters tenant … AND metric … AND time ….
2. time is DateTime64(3) holding the raw emission timestamp, so the GROUP BY groups almost nothing.

Combined, the projection is a worse-sorted, full-size copy of the table — and for a small tenant ClickHouse rejects it outright.

Evidence (ClickHouse 26.9.1, 100M-row synthetic table, 1.18 GiB, 201 tenants, whale = 60% of rows, 26-day window)

Verbatim, from --send_logs_level=debug on a small tenant:

<Debug> optimizeUseAggregateProjections: Projection p_cur is usable but requires
reading 344 marks, which is not better than the original table with 26 marks

Storage — same dim (clientName, cardinality 50), same data:

shape	rows	on disk
current (metric, time, tenant, dim)	11,232,000	76.98 MiB
this PR (tenant, metric, hour, dim)	162,500	263.85 KiB

The new shape is 162,500 rows at both 10M and 100M base rows — O(cardinality), not O(rows).

Read cost (SelectedMarks / SelectedRows, cache-independent):

tenant	shape	marks	rows read
small (0.2% of rows)	current — rejected, scans base table	25	204,800
small	this PR	2	16,384
whale (60% of rows)	current — projection used	336	2,752,512
whale	this PR	2	16,384

Wall clock on the same box (warm cache, 3 runs): small tenant 0.020/0.022/0.021 s → 0.014/0.016/0.016 s; whale 0.032/0.034/0.035 s → 0.017/0.017/0.017 s.

Projection-row count by dimension on the 100M table, i.e. what a dim would cost if added to the slate — the (tenant, metric, hour) floor is 127.5K rows:

dim	cardinality	projection rows
service	5	132.5K
country	100	162.5K
path	200	325.0K
resourceId	5000	3.13M

What changed

1. Projection shape. Events are now keyed (tenant, metric, toStartOfHour(time,'UTC'), …dims), built by a new addEventProjection(). Gauges are untouched — see below.

2. Read path. The projection stores the bucket, not raw time, so a predicate on time cannot be evaluated against it. findAggregatedFromTable() and getTimeSeriesFromTable() now re-express the time filters on the bucket — only where that is exactly equivalent, i.e. where the bounds already land on an hour boundary:

caller predicate	rewrite	exact?
time >= X, X on the hour	bucket >= X	yes
time < Y, Y on the hour	bucket < Y	yes
time <= Y, Y+1ms on the hour (e.g. 23:59:59.999)	bucket < Y+1ms	yes
between(X, Y), both as above	both of the above	yes
anything else (>= 13:37, <= now)	none — raw predicate kept	n/a

Verified on all 100M rows: with aligned bounds the totals are byte-identical (1,043,827,325 both ways; 490,351,576 both ways). With an unaligned bound the naive rewrites are wrong in both directions — 986,619,370 (narrower) or 990,706,150 (wider) against a true 988,160,900 — which is exactly why they are declined rather than approximated.

3. Coarser intervals compose over the bucket instead of nesting a subquery. toStartOfDay(time) does not match the projection key, but toStartOfDay(toStartOfHour(time,'UTC'),'UTC') does, and is value-identical: 0 mismatching rows out of 100M for 1h, 1d, 1w and 1M. Sub-hour intervals (1m/15m/30m) would need detail the projection has summed away — 50M/100M rows differ — so they keep reading raw time.

Why gauges are deliberately excluded

An earlier revision of this PR also reshaped the gauge projections. That was dropped, because the case for it does not survive contact with production:

• No measured problem. Over 24h in nyc3, grouping usage read timeouts by the table named in the failing SQL: 246 on projects_usage_events, zero on projects_usage_gauges. Over a 3h window /v1/usage/gauges served 402 requests, all HTTP 200 — roughly 27x the request volume of /v1/usage/events (15 in the same window), and completely healthy.
• No measured benefit. Gauges must keep raw time: argMax(value, time) orders on it, so bucketing it away would leave grouped argMax reads with nothing to order on. A gauge reshape therefore only reorders keys and aggregates nothing — in a migration fixture the reshaped gauge projections got slightly worse, 20.84 MiB → 21.72 MiB at an unchanged row count.
• Real cost. Four extra DROP + MATERIALIZE mutation pairs per region per table, each with its own read-degradation window, plus argMax semantics to re-reason about.

addProjection() is restored verbatim from main, and the events shape lives in a separate addEventProjection() rather than being switched by a flag through the shared one. So the gauge path has a zero-line diff, and the emitted gauge DDL is byte-identical — verified by running setup() on both origin/main and this branch against a live ClickHouse and diffing SHOW CREATE TABLE (1312 bytes, md5 181c0f67935c543feaa04b80d02960d5, identical). ClickHouseSchemaTest::testGaugeProjectionsAreLeftOnTheirOriginalShape pins the original (metric, time, tenant, dims) key order so this exclusion is not silently "finished" later without fresh measurements.

Caller audit — appwrite-labs/cloud and appwrite/server-ce

Every read call site of the library in both repos, and whether this PR can alter its result (not just its speed):

#	Call site	API	Time predicate	Affected?
C1	cloud Platform/Tasks/VerifyBilledUsage.php:822-824 (×3)	getTotalBatch / sumDailyBatch	>= from, < to, day-aligned	No — neither API goes through the rewritten paths
C2	cloud VerifyBilledUsage.php:873	getTotal (gauge)	< to only	No — gauge projections and gauge reads are both untouched
C3	cloud VerifyBilledUsage.php:891	find (gauge) + groupBy('resourceId')	< to only	No — gauge projections and gauge reads are both untouched
C4	cloud Billing/Workers/ProjectAggregation.php:1199	find (gauge)	[firstDay, lastDay) day-aligned	No — gauge; also currently commented out (CLO-4588)
S1	ce Usage/Http/Events/XList.php:203	sum (event)	>= start, <= end	No — sum() is untouched
S2	ce Usage/Http/Events/XList.php:232	find (event) + interval + dims	arbitrary user timestamps	Speed only. Bounds are unaligned in the default case, so the rewrite is declined and the SQL is unchanged. When a caller does pass hour-aligned bounds, the rewrite is exact
S3–S8	ce Usage/Http/Gauges/XList.php:192,200,220,259,373,459	find (gauge)	arbitrary	No — gauge
S9	ce Usage/Concurrency.php:64	findAcrossTenants (event) + groupBy('tenant') + 1m-family interval	5-minute aligned	No — 5m is sub-hour, excluded
S10–S11	ce Usage/Concurrency.php:112,137	findAcrossTenants (gauge)	>= now−168h	No — gauge
S12	ce Usage/Connection.php:110-112	findAcrossTenants ×2, findDaily	none	No — no time predicate; findDaily untouched
S13	ce Workers/Deletes.php:175	purge	none	No — untouched

Billing is provably unaffected. Every billing read (C1–C4) either uses an API this PR does not touch (getTotalBatch, sumDailyBatch, getTotal) or is a gauge read, and gauges are untouched by this PR. Nothing on the billing path reaches findAggregatedFromTable() or getTimeSeriesFromTable() on TYPE_EVENT. And even if it did, day-aligned midnight bounds are hour-aligned, so the rewrite would be the exact-equivalence case.

The only behaviour change available to any caller is a faster plan for an identical result. There is no opt-in flag because none is needed: the rewrite is declined rather than approximated whenever equivalence cannot be proven.

⚠️ Operator steps — this is a schema change

setup() is skipped in production (cloud's app/http.php provisioning block is inside if ($isProduction) { … return; }, and app/init/resources.php:799-805 wraps the adapter so Connection::setup() is a no-op). Production tables are provisioned by a separate out-of-band script, so nothing here migrates itself. ADD PROJECTION IF NOT EXISTS will silently keep the old definition on an existing table.

Per region, on the events table only (namespaced projects_* and console_* in cloud) — three projections, p_by_path, p_by_country, p_by_service:

-- 1. drop the old definitions
ALTER TABLE <db>.<events_table> DROP PROJECTION IF EXISTS p_by_path;
ALTER TABLE <db>.<events_table> DROP PROJECTION IF EXISTS p_by_country;
ALTER TABLE <db>.<events_table> DROP PROJECTION IF EXISTS p_by_service;

-- 2. re-add with the new shape (or just re-run the provisioning script)

-- 3. materialise over existing parts — ADD PROJECTION only covers new parts
ALTER TABLE <db>.<events_table> MATERIALIZE PROJECTION p_by_path
  SETTINGS mutations_sync = 0;
-- … repeat for p_by_country, p_by_service …

Gauge tables need no action at all. The migration script compares live projection definitions against what the library emits, and gauges now match byte-for-byte, so it will skip them on its own.

Watch system.mutations for completion. Step 3 rewrites every part of the events table, so run it off-peak; the new projections are ~300× smaller than the ones being dropped, so the steady-state disk and write-amplification cost goes down, not up.

Until step 3 completes, reads fall back to the base table — the same plan a small tenant already gets today, so there is no window where reads are worse than before.

Follow-up (deliberately not in this PR)

• Slate expansion. The UI asks for ~15 dimensions; only path, country, service are projected. The reshape makes a low-cardinality dim nearly free (127.5K floor → 162.5K for a cardinality-100 dim), so clientType, clientName, osName, deviceName, sdk, method+status are cheap on the read side. I did not measure the insert-side cost of carrying more projections, and that is the number that should decide the slate — so it belongs in its own PR with a write benchmark.
• Unaligned windows. Events/XList builds >= start / <= end from raw user timestamps, so console reads decline the rewrite and stay on the base table. Flooring those bounds to the hour in server-ce would let them route; alternatively the adapter could split the partial edge hours out and union them, the way sumHybridDailyAndRaw() already does for the daily MV.

Verification

• EXPLAIN projections=1 and system.query_log.projections (runtime evidence, not just the plan) assert routing in the tests.
• Day-granularity results are asserted bucket-for-bucket against a direct base-table scan with optimize_use_projections = 0, for both find(groupByInterval('1d')) and getTimeSeries('1d').
• Gauge projection DDL is asserted to keep its original (metric, time, tenant, dims) key order, and gauge reads to keep toStartOfHour out of their emitted SQL.
• phpstan: no errors. pint --test: pass.
• Full suite green in CI. Locally the 3 remaining ClickHouse failures and the DatabaseTest errors are environment-only (no MariaDB here) and are byte-identical to those on an unmodified origin/main checkout, compared by test ID rather than by count.

[greptile-apps]

Greptile Summary

The PR reshapes ClickHouse event projections around tenant-first hourly buckets and rewrites exactly aligned event-read predicates so those projections can serve grouped queries.

• Adds tenant-first hourly event projections while preserving existing gauge projections.
• Routes aligned event aggregations and time-series reads through the hourly bucket.
• Adds schema, routing, fallback, and result-equivalence coverage.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
src/Usage/Adapter/ClickHouse.php	Adds hourly event-projection DDL and exact-boundary query rewriting without an eligible follow-up defect.
tests/Usage/Adapter/ClickHouseDimRoutingTest.php	Expands integration coverage for aligned projection routing, unaligned and sub-hour fallback, gauge isolation, and aggregate equivalence.
tests/Usage/Adapter/ClickHouseSchemaTest.php	Pins the new tenant-first event projection shape and confirms gauge projection DDL remains unchanged.

_{Reviews (4): Last reviewed commit: "test(clickhouse): cover 1w and 1M bucket..." | Re-trigger Greptile}

[Copilot]

Pull request overview

Reshapes ClickHouse event projections for efficient tenant-first, hourly aggregation while preserving exact query results.

Changes:

• Adds tenant-first hourly event projections while leaving gauges unchanged.
• Rewrites aligned time predicates to use hourly buckets.
• Adds schema, routing, fallback, and result-parity tests.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File	Description
src/Usage/Adapter/ClickHouse.php	Implements event projection schema and aligned bucket routing.
tests/Usage/Adapter/ClickHouseSchemaTest.php	Verifies event and gauge projection shapes.
tests/Usage/Adapter/ClickHouseDimRoutingTest.php	Tests projection routing, fallback, and result parity.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[lohanidamodar]

Good catch on the coverage gap — added in 89e7bb3, which turns the day-interval test into a provider over 1d/1w/1M so each asserts both projection use and parity against a base-table scan.

On correctness the composition was already sound: toStartOfInterval(toStartOfHour(t,'UTC'), INTERVAL X) equals toStartOfInterval(t, INTERVAL X) with zero mismatches across 5M timestamps spanning ~5 years, for 1h/1d/1w/1M. Week and month boundaries are whole numbers of hours, so hour truncation nests inside them. But the suite was proving that only for 1d, so the guard was worth having.