Skip to content

feat(usage): peak concurrent connections aggregation (CLO-4542)#22

Draft
lohanidamodar wants to merge 2 commits into
mainfrom
clo-4542
Draft

feat(usage): peak concurrent connections aggregation (CLO-4542)#22
lohanidamodar wants to merge 2 commits into
mainfrom
clo-4542

Conversation

@lohanidamodar

@lohanidamodar lohanidamodar commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

CLO-4542 — Peak concurrent realtime connections (query-side)

realtime.connections is emitted as +1 on connect / -1 on disconnect and stored as a TYPE_EVENT, aggregated with SUM. A plain MAX(value) over ±1 rows is meaningless (returns 1), and a per-pod gauge snapshot can't give a project-wide peak. The ±1 deltas already contain every connect/disconnect across all pods with timestamps, so peak concurrency is exactly max(running_sum(value)) ordered by time — cross-pod correct, no producer change (approach B).

Changes

UsageQuery — new aggregate query hint mirroring groupBy:

  • TYPE_AGGREGATE, VALID_AGGREGATES = ['sum','peak'] (sum = current default).
  • aggregate(string $function): self (validates), plus isAggregate / extractAggregate / removeAggregate helpers and isMethod() support.

ClickHouse adapter:

  • parseQueries() records aggregate=peak and splits the time filters from the non-time filters, capturing the window-start param so the baseline subquery can key on time < start. The sum/absent path is 100% unchanged.
  • findFromTable() routes peak to a new findPeakFromTable() (before the SUM-based aggregated path) which builds:
SELECT metric, max(running) as value, toStartOfInterval(time, {interval}) AS bucket
FROM (
  SELECT metric, time,
         ifNull((SELECT sum(value) FROM t WHERE {nonTimeFilters} AND time < {start}), 0)
           + sum(value) OVER (PARTITION BY metric[, tenant][, dims] ORDER BY time ASC
                              ROWS BETWEEN UNBOUNDED PRECEDING AND CURRENT ROW) AS running
  FROM t WHERE {nonTimeFilters} AND time >= {start} AND time <= {end}
)
GROUP BY metric, bucket ORDER BY bucket ASC

Without groupByInterval the bucket column is dropped → a single max(running) row per metric. Honours interval bucketing, groupBy dimensions, and limit/offset/orderBy consistently with findAggregatedFromTable. 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 at start) 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 UsageQuery unit coverage for aggregate() + 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 -1 disconnects and folded sub-flush bursts away):

  1. Opt-in negative deltas (strict by default). Negative values stay rejected by default for every metric, events included, so a buggy negative count/bandwidth is still caught. A caller emitting a genuine signed delta opts in per row:
    • Accumulator::collect(..., ?int $foldSeconds = null, bool $allowNegative = false) — the flag rides onto the buffered entry and is handed to addBatch.
    • ClickHouse::validateMetricData(..., bool $allowNegative = false) gates the guard (if ($value < 0 && !$allowNegative) throw); validateMetricsBatch() reads allowNegative off each row.
    • The flag is validation-only — it is never written as a stored column. The library stays generic; the cloud worker passes allowNegative: true (with foldSeconds: 1) for realtime.connections.
  2. Optional per-second fold. When foldSeconds is set, the second bucket floor(ts / foldSeconds) * foldSeconds 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. When null, behaviour is identical to before.

Tests: negatives rejected by default via both collect() and addBatch; persisted and netted when opted in; gauges still reject; per-second fold groups within / splits across seconds; and aggregate('peak') over per-second net rows returns the true peak (4) for a burst that a single per-flush net row hides (peak 1).

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.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant