Skip to content

fix(metrics): bound latency segment memory by summing into fixed time buckets - #398

Open
LeeroyHannigan wants to merge 2 commits into
mainfrom
fix/bounded-latency-segments
Open

LeeroyHannigan wants to merge 2 commits into
mainfrom
fix/bounded-latency-segments

Conversation

@LeeroyHannigan

Copy link
Copy Markdown
Collaborator

What

MetricsCollector kept one SegmentPoint per data-plane request for 24 hours: a 112-byte struct plus two heap strings, pushed to a Vec on every request and trimmed only by the five-minute prune. The console's Latency Breakdown and the segments array of /metrics read nothing but a per-operation count and the mean of the six latency segments over a window, so the points are now summed at record time.

  • New crates/core/src/metrics/segments.rs: each operation owns two fixed rings of 64-byte buckets, 301 one-second buckets serving LastMinute and Last5Minutes and 1,441 one-minute buckets serving LastHour, LastDay and AllTime. A bucket holds a count and six sums. Slot k of a ring of width b covers elapsed seconds [k*b, (k+1)*b) since the collector's epoch and lives at position k % len; a record into a position whose stored slot is older resets it in place.
  • A record is one hash lookup and two bucket updates under the existing write lock, with no allocation after the first record of each operation. The clock is read under the lock on both the record and the query path, so a stored record is never ahead of a query that follows it.
  • Memory is at most operations * 111,488 bytes. The handler rejects any X-Amz-Target outside is_known_operation before any metric is recorded, so operations is at most the 37 names in the dispatch table: under 4 MiB per process, independent of request rate, table count and uptime.
  • query_segments and the OperationSegments JSON shape are unchanged; the console needs no change. Per-operation counts are exact and the means are the same sums divided by the same counts as before (for integer microsecond inputs the f64 sums stay exact below 2^53). Operations with no request in the window are omitted, as before; results are now sorted by operation name.
  • prune() no longer touches segments; buckets are overwritten as time advances.

What changes for a reader: a window of W seconds returns at least the last W seconds and at most W plus one bucket width, so LastMinute and Last5Minutes may include up to 1 s beyond the window and LastHour and LastDay up to 60 s. AllTime returns the whole 24-hour ring (before, it meant since the last five-minute prune). The console refreshes every 10 s on Last5Minutes and shows means and a count, so this is not visible there.

The table name is no longer passed to record_segments. Nothing read it (the field carried a dead_code allow and a TODO), and a per-table dimension would multiply the bound by the table count, which clients control. SuccessfulRequestLatency in the persisted counter metrics still carries the table and operation dimensions.

The issue asks for a granularity, retention, sampling or cap setting and an overflow counter. None is added: the structure is bounded by the data model rather than by a budget, so there is nothing for a setting to protect. A configuration key would also route through the RFC process and delay the fix.

Why

Closes #397. On a PostgreSQL deployment of 0.1.12 at 274 requests per second, process memory reached about 4.3 GiB in 24 hours (the reporter's measurement, 23.64 million requests) inside a 6 GiB container limit. Reproduced here on the unmodified 0.1.13 release binary: 1.2 million GetItem at 1,968 per second over 10 minutes grew anonymous memory by 193.5 bytes per request (least squares from 180 s, r² 0.998); the same build with record_segments made a no-op was flat at 0.6 bytes per request (r² 0.003), so the segment vector was the only per-request growth. 193.5 bytes times the reporter's 23.64 million requests is 4.26 GiB. PR #386 named the 24-hour segments as separate work.

The vector exists at every release through 0.1.13. Until a deployment upgrades, the deployment guide now gives the growth law (about 190 bytes per data-plane request over a trailing 24 hours, the reporter's figure) and a sizing rule.

Result

Measured on this host (16 cores, PostgreSQL 15 in a container, release build, GetItem on an 8-item table at 1,984 per second for 20 minutes, 2.4 million requests, 0 errors):

0.1.13 this PR
Anonymous memory growth, least squares from 180 s 193.5 B/request, r² 0.998 5.8 B/request, r² 0.48
Same, from 600 s not measured (10-minute run) 1.7 B/request, r² 0.04
Anonymous memory at 10 min / 20 min 312 MB at 10 min 92.6 MB / 93.1 MB
Segment store after the run 1.2 million points at 176 bytes each (modelled), about 211 MB 5 operations × 111,488 bytes, 557 KB

The remaining movement is the one-minute counter flush: from 300 s on, anonymous memory oscillated between 88 and 103 MB with per-minute deltas alternating in sign. The /metrics segments count for GetItem over LastDay was 2,400,033 against 2,400,032 driver requests plus the one GetItem the table setup issues.

Microbenchmarks, release profile, single thread, 1 million calls, three runs each: record 73 ns per call against 176 ns for the old push (the old path allocated two strings per request); query_segments(LastDay) with 37 operations holds the read lock for 39 µs (p50, 5,000 samples), against 39.3 to 39.5 ms for the old full scan over 1 million points, during which every request's record waited. Under lock saturation the lock, not the work, is the limit for both shapes: at 8 writer threads the old push ran 2.3 to 4.5 million records per second and the new record 2.0 to 3.1 million (the hash of the operation name now runs under the lock, where the old path pushed a prebuilt point); at 16 threads both ran about 2 million. The live run above recorded 1,984 per second, three orders of magnitude below that.

Testing done

  • crates/core/src/metrics/segments_tests.rs (new): bucket size pinned at 64 bytes; boundedness over five million synthetic requests across five operations and three simulated days asserting the bucket count equals operations × 1,742; a random workload of 300,000 points compared against a naive per-point reference for count (exact) and means (1e-9 relative) over four windows and four query instants; one request per second for three laps of the coarse ring checked against a closed-form arithmetic series at every ring and window boundary; window filters at second and minute boundaries; ring wrap and stale-slot exclusion; operations sorted and zero-count operations absent; an ignored timing comparison of the old push against the new record. collector_tests.rs adapted to the new signature, with the prune test rewritten to age points through the ring.
  • Negative controls: with the ring position computed without the modulo (one bucket per slot forever), boundedness_five_million_records and ring_lap_and_stale_slots fail; with the window filter removed from the bucket sum, six tests fail (random_workload_matches_naive_reference, window_filter_at_minute_boundary, fine_windows_cut_at_the_second, ring_lap_and_stale_slots, operations_sorted_and_zero_count_absent, prune_leaves_segments_alone_and_the_ring_ages_them_out).
  • tests/test_metrics.py, tests/test_console_integration.py, tests/test_console_cache_coherence.py against a release build of this branch: 42 passed.
  • Live run as above: 20 minutes at 1,984 GetItem per second, /metrics counts for LastDay, Last5Minutes and LastMinute consistent with the driver count and the rate.
cargo fmt --all -- --check
cargo clippy --all-targets -- -D warnings              # clean on 1.97.0 (the CI pin) and 1.88.0 for extenddb-core
cargo clippy -- -W clippy::pedantic -W clippy::unwrap_used -W clippy::expect_used   # nothing new in the changed files
cargo test --workspace                                 # 1,275 passed, 0 failed, stable
rustup run 1.88.0 cargo test -p extenddb-core          # 514 passed
cargo doc -p extenddb-core --no-deps                   # no new warnings

Docs: docs/design/06-component-server.md section 7.2 states the bucket widths and what each window returns; docs/manuals/11-deployment-guide.md gains a Process Memory subsection for operators on releases through 0.1.13; docs/todo-index.md drops the entry for the removed field; the metrics module doc notes that segments need no prune.

Checklist

  • I have read CONTRIBUTING.md
  • All tests pass (cargo test --workspace)
  • Code is formatted (cargo fmt --check)
  • Clippy is clean (cargo clippy -- -W clippy::pedantic)
  • I have added or updated tests for new functionality
  • I have updated documentation if behavior changed
  • Breaking changes are noted below (if any)
  • If this changes the wire protocol, Storage trait, auth model, on-disk
    format, or public CLI surface, an RFC has been accepted or is linked
    below. Otherwise, an ADR captures the decision (link below).

ADR / RFC: n/a. Internal to the in-memory metrics collector; no wire, trait, auth, on-disk, config or CLI change.

Breaking changes

extenddb_core::metrics::MetricsCollector::record_segments changed from (&self, operation: &str, table_name: Option<&str>, segments: LatencySegments) to (&self, operation: &str, segments: &LatencySegments). The one caller in the workspace (crates/server/src/handler.rs) is updated; extenddb-core is not published. No wire, CLI, configuration or on-disk surface changed.


By submitting this pull request, I confirm that my contribution is made under
the terms of the Apache License 2.0 and I agree to the Developer Certificate of
Origin (DCO). See CONTRIBUTING.md for details.

… keeping one point per request

MetricsCollector kept a SegmentPoint (112 bytes plus two heap strings) for
every data-plane request for 24 hours. At 274 requests per second that is
23.7 million points a day and about 4.3 GiB of anonymous memory (issue
#397); measured here, the 0.1.13 binary grows by 193.5 bytes per GetItem.
The console only reads per-operation counts and mean segments over a
window, so the points are summed at record time instead.

Each operation now owns two fixed rings of 64-byte buckets: 301 one-second
buckets serving LastMinute and Last5Minutes, and 1441 one-minute buckets
serving LastHour, LastDay and AllTime. A record is one hash lookup and two
bucket updates under the write lock; the rings are allocated once per
operation. Memory is at most operations * 111,488 bytes, and the server
only records the 37 operations in the dispatch table, so under 4 MiB per
process. A window of W seconds covers at least the last W seconds and at
most W plus one bucket width. The clock is read under the lock on both the
record and the query path, so a stored record is never ahead of a query
that follows it. prune() no longer touches segments.

record_segments(operation, &LatencySegments) no longer takes the table
name: a per-table dimension would multiply the bound by the table count,
which clients control, and nothing read it. The one caller is updated.

Docs: the design doc states the bucket widths and what each window returns;
the deployment guide gains a Process Memory subsection with the growth law
for releases through 0.1.13; the todo-index entry for the table field is
removed.

Tests: boundedness over five million synthetic requests, a naive-reference
comparison on a random workload, continuous load across three laps of the
coarse ring against a closed-form reference, window filters at second and
minute boundaries, ring wrap and stale-slot exclusion, and an ignored
timing comparison of the old push against the new record.

This branch has not been deployed

No deployments
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.

[Bug] Bound in-memory latency segment retention under sustained request rate

1 participant