feat(usage): peak concurrent connections aggregation (CLO-4542)#22
Draft
lohanidamodar wants to merge 2 commits into
Draft
feat(usage): peak concurrent connections aggregation (CLO-4542)#22lohanidamodar wants to merge 2 commits into
lohanidamodar wants to merge 2 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.
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 — Peak concurrent realtime connections (query-side)
realtime.connectionsis emitted as+1on connect /-1on disconnect and stored as aTYPE_EVENT, aggregated withSUM. A plainMAX(value)over±1rows is meaningless (returns1), and a per-pod gauge snapshot can't give a project-wide peak. The±1deltas already contain every connect/disconnect across all pods with timestamps, so peak concurrency is exactlymax(running_sum(value))ordered by time — cross-pod correct, no producer change (approach B).Changes
UsageQuery— newaggregatequery hint mirroringgroupBy:TYPE_AGGREGATE,VALID_AGGREGATES = ['sum','peak'](sum= current default).aggregate(string $function): self(validates), plusisAggregate/extractAggregate/removeAggregatehelpers andisMethod()support.ClickHouseadapter:parseQueries()recordsaggregate=peakand splits the time filters from the non-time filters, capturing the window-start param so the baseline subquery can key ontime < start. Thesum/absent path is 100% unchanged.findFromTable()routespeakto a newfindPeakFromTable()(before the SUM-based aggregated path) which builds:Without
groupByIntervalthebucketcolumn is dropped → a singlemax(running)row per metric. Honours interval bucketing,groupBydimensions, andlimit/offset/orderByconsistently withfindAggregatedFromTable. Injection-safe via the existing{name:Type}parameter binding.Baseline note
The baseline scalar subquery (
time < start) adds connections opened before the window (still open atstart) so an in-window peak isn't understated. It scans history before the window; for billing windows this is bounded by monthly partition pruning but could be non-trivial on very old projects. It can be bounded later (e.g.time >= start - 1 day) at a small accuracy cost if it proves costly. Default: correct/unbounded baseline.Tests
Flat and interval peak, pre-window baseline inclusion, interleaved-"pod" sum-before-max, and
UsageQueryunit coverage foraggregate()+ helpers.Draft: appwrite/cloud depends on a subsequent tagged release.
Update — write-path correctness (opt-in
-1, per-second fold)Two follow-ups so the peak is correct end-to-end (the write path previously dropped
-1disconnects and folded sub-flush bursts away):Accumulator::collect(..., ?int $foldSeconds = null, bool $allowNegative = false)— the flag rides onto the buffered entry and is handed toaddBatch.ClickHouse::validateMetricData(..., bool $allowNegative = false)gates the guard (if ($value < 0 && !$allowNegative) throw);validateMetricsBatch()readsallowNegativeoff each row.allowNegative: true(withfoldSeconds: 1) forrealtime.connections.foldSecondsis set, the second bucketfloor(ts / foldSeconds) * foldSecondsjoins 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 identical to before.Tests: negatives rejected by default via both
collect()andaddBatch; persisted and netted when opted in; gauges still reject; per-second fold groups within / splits across seconds; andaggregate('peak')over per-second net rows returns the true peak (4) for a burst that a single per-flush net row hides (peak 1).