Skip to content

(13) stats - #1774

Draft
daniel-noland wants to merge 26 commits into
pr/daniel-noland/driven-clockfrom
pr/daniel-noland/stats-collector
Draft

daniel-noland wants to merge 26 commits into
pr/daniel-noland/driven-clockfrom
pr/daniel-noland/stats-collector

Conversation

@daniel-noland

@daniel-noland daniel-noland commented Aug 28, 2026

Copy link
Copy Markdown
Collaborator

No description provided.

@coderabbitai

coderabbitai Bot commented Aug 28, 2026

Copy link
Copy Markdown

Important

Draft PR not reviewed

Draft PRs are not automatically reviewed by default.

  • Trigger a manual review

To automatically review draft PRs, update your CodeRabbit configuration:

reviews:
  auto_review:
    drafts: true

Comment @coderabbitai help to get the list of available commands.

@daniel-noland daniel-noland added the ci:+merge-ready Run all checks which will be run in the merge queue regardless of label status label Aug 28, 2026
@codecov

codecov Bot commented Aug 28, 2026

Copy link
Copy Markdown

@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from fa0e8f2 to b164a0f Compare August 28, 2026 03:05
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch 2 times, most recently from 80e27f5 to c14f713 Compare August 28, 2026 03:34
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from b164a0f to 3cd5687 Compare August 28, 2026 03:34
@daniel-noland daniel-noland changed the title fix(stats): the collector's rate, its windows, and what happens when a VNI changes hands stats Aug 28, 2026
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from c14f713 to 42a24de Compare August 28, 2026 04:22
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch 2 times, most recently from 1ec0be0 to de8e45f Compare August 28, 2026 05:11
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 42a24de to 39a7f24 Compare August 28, 2026 05:11
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from de8e45f to bea6549 Compare August 28, 2026 05:32
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 39a7f24 to 05f602f Compare August 28, 2026 05:32
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from bea6549 to f850a96 Compare August 28, 2026 05:48
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 05f602f to 7823471 Compare August 28, 2026 05:48
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from f850a96 to 9398527 Compare August 28, 2026 06:21
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 7823471 to b6054e3 Compare August 28, 2026 06:23
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from 9398527 to d497a39 Compare August 28, 2026 06:40
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from b6054e3 to 4d2a746 Compare August 28, 2026 06:40
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from d497a39 to 3e43aea Compare August 28, 2026 07:08
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 4d2a746 to a68c341 Compare August 28, 2026 07:08
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from 3e43aea to 610dcf1 Compare August 28, 2026 07:31
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from a68c341 to 2064af0 Compare August 28, 2026 07:31
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from 610dcf1 to 698f663 Compare August 28, 2026 07:43
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 2064af0 to c620950 Compare August 28, 2026 07:44
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from 698f663 to 26b967b Compare August 28, 2026 09:14
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from c620950 to 21dc552 Compare August 28, 2026 09:14
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/driven-clock branch from 26b967b to 8968292 Compare August 28, 2026 17:16
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from 21dc552 to 7d29a55 Compare August 28, 2026 17:16
@daniel-noland daniel-noland added ci:+test/all-profiles run tests under all profiles ci:+sanitize Run the address and thread sanitizer jobs on this PR ci:+concurrency Run the shuttle and loom concurrency jobs on this PR labels Sep 6, 2026
@daniel-noland
daniel-noland force-pushed the pr/daniel-noland/stats-collector branch from c2d6db2 to 46219f1 Compare September 6, 2026 18:30
daniel-noland and others added 26 commits September 9, 2026 12:48
Savitzky-Golay edge coefficients can drive a sparse window below zero,
so low traffic appeared as a negative CLI rate. A sample step that
rounded to zero microseconds divided by zero and returned `Ok(NaN)`,
which callers treated as valid.

Clamp smoothed counts at zero, reject zero-length steps, and assert
that successful rate calculations are finite and non-negative.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The spec-to-interface property required full equality, but documented
matching treats an absent MAC or MTU as an OS-chosen wildcard. Correct
lenient matches therefore failed the property.

Assert required fields individually and check optional fields only
when the spec supplies them. This defines matching without reusing the
comparator under test.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A ring buffer's physical order stops matching arrival order after it
wraps, but each smoothing coefficient depends on position. Constant
load therefore produced a wrong rate on four ticks in five, sometimes
reporting zero. Missing destinations also repeated their previous
sample instead of representing an idle interval.

Read samples from oldest to newest and insert zero for destinations
absent from an interval.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The CLI reads live dataplane structures while workers forward traffic
and apply new configurations. A lock cycle there can freeze both the
dataplane and the diagnostics needed to explain the original failure.

Add a model property that performs those operations concurrently and
distinguishes a provider panic from a lack of progress.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Rate tests only checked that calculation returned `Ok`, allowing
reordered windows and negative values to reach production.

Drive the collector clock and compare published packet and byte rates
with known steady and ramp loads. The steady case pins scaling; the
ramp makes sample order observable.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Exact-oracle tests always placed a window at ring position zero, while
wrapped-window properties checked only its sign. The chronological
ordering defect lived between those two test sets.

Carry generated polynomial windows through arbitrary ring offsets and
compare the stencil with a two-point derivative wherever both should
be exact.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Startup created ten copies of one batch window because its scan state
never advanced. An update between those duplicate windows matched none
and disappeared permanently from cumulative counters.

Open consecutive windows, assign unmatched traffic to the earliest
open batch instead of dropping it, and share one apportionment helper
between updates and finalization.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Fixed counter properties placed every update in the tick that had just
ended. They could not exercise delayed or skewed arrivals, including
the gap that lost a startup tick and the timing produced by a stalled
collector.

Draw each update's time window independently and check the ledger
against the total traffic supplied.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Smoothing properties generated running totals, while the collector
supplies per-interval counts. Millions of generated windows never
decreased, contained zero, or smoothed below zero, so the clamp tests
passed even with the clamp removed.

Generate interval counts and require evidence that negative smoothing
cases were reached. Keep the claim scoped to `Smooth`; the derivative
and EWMA implementations have no production caller.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Renaming a VPC kept its discriminant but changed its metric labels, so
old series remained beside the new live series. Removal also paired a
departed VPC only with survivors, leaving both ends of a deleted
peering uncleared. Tests missed both because no metrics recorder was
installed.

Reconcile names with discriminants, retire the correct series, and
install a recorder that makes gauge updates observable in tests.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Every update re-registered gauges for all VPC pairs and materialized a
store entry even when a pair had never carried traffic. With 64 VPCs
and one active pair, that meant 33,280 registrations per tick and
4,096 stored pairs. The extra work consumed the bounded channel's
service time when statistics mattered most.

Register series on configuration changes and create or publish pair
state only after traffic exists.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A full collector channel discarded a delta batch, permanently
understating cumulative counters with only a warning to reveal it.

Keep the unsent batch and retry it with `try_send_option`. Batches are
keyed by VPC pair, so waiting longer does not increase memory use.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The reader skipped the packet pipeline after empty reads, so a quiet
interface never let the timed statistics stage observe its deadline.
Its last batch waited for new traffic and then included the entire idle
gap in the rate window.

Run an empty pipeline pass on the existing watchdog tick. This gives
each interface a clock edge without adding another wakeup.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
A VNI handed directly from one tenant to another never left the map, so
pruning did not reset it. The incoming tenant's first scrape included
every packet counted for the former owner. Separate registration paths
also built inconsistent labels and could duplicate `from`.

Treat an owner-name change as a reset and centralize metric label
construction.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Resetting the store left the outgoing tenant's traffic in stage
batches, pending collector batches, and smoothing windows keyed only by
VNI. Those values arrived after handover and were credited to the new
owner.

Remove a recycled VNI from every in-flight layer, both as a traffic
source and as a destination.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Pruning, rate calculation, and store creation all walked every possible
VPC pair, making collector time and memory quadratic in the configured
fabric. At 64 VPCs with one active pair, one batch took 159 ms.

Process only concluded and active pairs, reducing that case to 1.6 ms
and one stored pair. Publish counter removal before new names so a
transitional read cannot label old traffic as the new tenant.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Deleting a destination should retire the pair series, but it also
removed the surviving source VPC's in-flight traffic from its lifetime
total. A failed VPC-map read was likewise treated as an empty map and
triggered a destructive prune.

Account source totals independently of destination lifetime and
distinguish an unavailable map from a genuinely empty configuration.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Names, VPC counters, and pair counters lived behind separate locks and
were read one at a time. During a handover, a reader could combine one
tenant's name with another tenant's traffic regardless of writer order.

Take all three read guards in a consistent order and expose one
snapshot to management. Combine counter invalidation and name updates
in one handover operation as well.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Advancing virtual time woke NAT timer tasks but returned before their
multi-poll teardown released the flow tables. Four fixed yields still
let retained state grow across fuzz cases until the process ran out of
memory.

Wait for the live-task count to reach zero, with a bounded timeout that
fails any case whose timers cannot retire.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The flow-entry fuzz runtime lived for the entire campaign, and its timer
tasks released their case state only when that runtime was dropped.
Retained state grew by about 175 KB per case and slowed the target as
its queue expanded.

Construct and drop the runtime inside each case so its timers and flow
tables cannot accumulate across inputs.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
DPDK rejects duplicate ACL context names, but the concurrency facade's
atomic belongs to a model execution and cannot provide a process-wide
sequence under Loom or Shuttle. Model tests therefore reused names and
failed while an earlier context was still live.

Use the facade atomic normally and a real process-global atomic under
model-checking backends, matching the flow-filter naming policy.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Seven of eleven model properties explored schedules over one fixed
input shape. They could not vary flow counts, hosts, ports, public
address capacity, or the number of operations around concurrent
traffic.

Draw those inputs for both model-checking and plain test backends. Keep
barrier round counts fixed where all three participants must rendezvous
equally, avoiding a generated deadlock unrelated to the property.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
The flow-info property compared an untouched partner with the generated
genid. When both were zero, it reported a failure on correct behavior.

Record the partner's value before the operation and assert that it
remains unchanged, giving every generated genid the same meaningful
oracle.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Input generation left two doc comments unattached, used a one-pattern
`match`, and placed constants below runtime bindings. The all-targets
Clippy job rejects all three forms.

Remove the stale comments, use `let ... else`, and move constants to
the start of each property without changing behavior.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Statistics code and tests still imported synchronization primitives
directly from `std`, bypassing the crate's concurrency backend.

Use the concurrency facade for model-visible locks, atomics, and
pointers. Keep process-unique external-library counters as explicit
standard-library exceptions.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Under the Loom feature, the concurrency facade's `Arc` becomes a Loom
pointer. The third-party metrics crate expects `std::sync::Arc`, so the
test recorder no longer compiled.

Allocate gauge handles with `concurrency::process_global::Arc`. The
recorder is installed once for the process and lies outside the model,
which is exactly the process-global boundary.

Signed-off-by: Daniel Noland <daniel@githedgehog.com>
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ci:+concurrency Run the shuttle and loom concurrency jobs on this PR ci:+merge-ready Run all checks which will be run in the merge queue regardless of label status ci:+sanitize Run the address and thread sanitizer jobs on this PR ci:+test/all-profiles run tests under all profiles

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant