feat(usage): signed deltas, cross-tenant reads and max aggregation (CLO-4542) - #22
Draft
lohanidamodar wants to merge 4 commits into
Draft
feat(usage): signed deltas, cross-tenant reads and max aggregation (CLO-4542)#22lohanidamodar wants to merge 4 commits into
lohanidamodar wants to merge 4 commits into
Conversation
Realtime connections are stored as +1/-1 event deltas aggregated with SUM, so a plain MAX(value) is meaningless. Add an `aggregate` query hint (sum | peak) mirroring the groupBy pattern. `peak` computes the peak concurrent value as max(running_sum(value)) ordered by time, cross-pod correct with no producer change. - UsageQuery: TYPE_AGGREGATE, VALID_AGGREGATES, aggregate() plus isAggregate/extractAggregate/removeAggregate helpers. - ClickHouse: parseQueries splits time vs non-time filters and records the window-start param for peak; findFromTable routes peak to a new findPeakFromTable that builds a windowed running-sum with a pre-window baseline subquery (connections still open at start). Honours interval bucketing, dimensions, limit/offset/orderBy. The sum/default path is unchanged. - Tests: flat + interval peak, pre-window baseline, interleaved-producer sum-before-max, and UsageQuery unit coverage.
Make the peak path correct end-to-end for delta metrics like realtime connections, where the write side previously dropped -1 disconnects and folded sub-flush bursts away. - Reject negative values by default for every metric (events included) so a buggy negative count/bandwidth is still caught. Callers emitting a genuine signed delta opt in per row via allowNegative: Accumulator::collect(..., bool $allowNegative = false) carries the flag onto the buffered entry and hands it to addBatch; ClickHouse::validateMetricData(..., bool $allowNegative = false) gates the guard, read from each row's `allowNegative` in validateMetricsBatch. The flag is validation-only — never written as a column. The library stays generic; the caller decides which metrics may be negative. - Add optional foldSeconds to Accumulator::collect(). When set, the second bucket (floor(ts / foldSeconds) * foldSeconds) joins the fold key and becomes the entry time, so events fold only within the same bucket and intra-flush peaks survive. When null, behaviour is unchanged. - Tests: negatives rejected by default (collect + addBatch), persisted and netted when opted in, gauges still reject; per-second fold groups/splits by second; peak over per-second net rows captures a burst a per-flush net hides.
findAcrossTenants() applies no tenant filter so an aggregation job can roll
every tenant up in one pass instead of issuing N per-tenant queries. Shared
tables only; groupBy('tenant') keeps the rows attributable.
parseQueries() now takes a nullable tenant - null is the explicit cross-tenant
read, '' still fails fast so an empty scope can never be read silently.
The peak concurrency figure is now pre-computed into a gauge level series by
an operator-side job, so nothing reads a running-sum max at request time. Drop
findPeakFromTable and its baseline plumbing, and keep the aggregate hint for
what does need it: rolling a gauge series up to a coarser interval.
Gauges default to argMax(value, time) - the latest reading in the bucket, right
for a snapshot but wrong for a sampled level series, where the bucket's highest
sample is the answer. aggregate('max') selects that.
Removing the peak path also removes its baseline defect: the pre-window
baseline was a single non-correlated scalar added to a running sum partitioned
by metric[, tenant][, dims], so any dimension break-down had every series
inflated by the combined baseline of all of them.
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
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.
CLO-4542 — support for peak concurrent realtime connections
realtime.connectionsis a delta metric:+1on connect,-1on disconnect, emitted by every realtime pod. Concurrency is therefore the cumulative sum of those deltas — summing them over a window gives the net change (roughly zero for a balanced window), andMAX(value)gives1.Cloud pre-computes that level in
StatsResourcesand stores it as a gauge (appwrite-labs/cloud#4662). This PR is the library support that job needs.Changes
Signed deltas, strict by default —
Accumulator::collect()andClickHouse::validateMetricData()still reject negative values for every metric, so a buggy negative count or bandwidth figure is still caught. A caller emitting a genuine signed delta opts in per row withallowNegative: true. The flag is validation-only and never stored as a column; the library stays generic and holds no metric names.Cross-tenant reads —
findAcrossTenants(array $queries, ?string $type)applies no tenant filter, so an operator-side aggregation job can roll every tenant up in one pass instead of issuing N per-tenant queries. Shared tables only.tenantis now accepted as agroupBydimension so the returned rows stay attributable.This is deliberately a separate method rather than a nullable
$tenantonfind().find()keeps its non-nullable signature, and only the internals (findScoped,findFromTable,parseQueries) take?string, so the unscoped path can't be reached without naming it. A nullable public parameter would mean anullarriving from an uninitialised project id silently returns every tenant's rows; as a named method it can't happen by accident and every cross-tenant read is greppable. NoteparseQueriesstill rejects''— an empty tenant remains a fail-fast error, not a wildcard.aggregate('max')— a query hint that overrides the per-type default value expression. Gauges default toargMax(value, time), the latest reading in the bucket. That's right for a snapshot such as storage, but wrong for a sampled level series, where the bucket's highest reading is the answer.maxcomposes, so the max of 5-minute samples is the hourly, daily or billing-period peak.Optional per-second fold — when
foldSecondsis set, the second bucket joins the fold key and becomes the entry time, so events fold only within the same window and intra-flush bursts survive as per-second net rows. Whennull, behaviour is unchanged. Cloud no longer enables this (5-minute sampling makes per-second resolution moot), but the mechanism is generic and tested, so it stays.Superseded
findPeakFromTableandaggregate('peak')are gone. Nothing reads a running-sum max at request time once the level is pre-computed, and the query was expensive on the path that would have used it — it needed every delta since the project began.Removing it also removed a defect. The pre-window baseline was a single non-correlated scalar:
added to a running sum partitioned by
metric[, tenant][, dims]. Those only agree when the window covers exactly one series. Any dimension break-down —aggregate('peak')withgroupBy('country')— had every series inflated by the combined baseline of all of them. The peak tests only coveredgroupByInterval, never a dimension, so nothing caught it.Tests
ClickHouseGaugeMaxTestreplacesClickHousePeakTest:lastvsmaxon the same data, per-bucket maxima, the flat single-row shape billing uses, tenant-scopedmaxunder shared tables,findAcrossTenantsreturning a row per tenant, and the non-shared-tables rejection.AccumulatorTestcovers negatives rejected by default through bothcollect()andaddBatch, persisted and netted when opted in, gauges still rejecting, and the per-second fold grouping and splitting.UsageQueryTestcovers theaggregatehint and helpers.Not yet run locally — no composer on this machine, and the ClickHouse suite needs a live instance. Relying on CI.
Merge order
Cloud depends on a tagged release of this. Draft until then.