fix(crdt): cap distinct origin site-slots per tenant_spend counter - #63
Merged
nerdalert merged 2 commits intoAug 16, 2026
Merged
Conversation
grid#47 stopped pruning tenant_spend on SWIM membership eviction (a site marked Suspect/Dead was wiping its own contribution, letting an exhausted budget reopen on a transient partition or restart). That fix traded away the growth bound the buggy behavior accidentally provided: nothing now removes a site's slot from a tenant's GCounter once recorded, so a compromised or churning origin repeatedly claiming new site identities could grow a single tenant's counter without bound. MAX_TRACKED_TENANTS already bounds distinct tenant_id keys; this adds MAX_TENANT_SPEND_ORIGINS as an independent bound on distinct site-slots within a single tenant's counter, enforced in merge_tenant_spend_from_origin: once a tenant's counter has that many origins, a brand-new origin's contribution is refused, while already-tracked origins keep accepting updates. Mirrors the existing MAX_TRACKED_TENANTS bound's shape exactly. Adds GCounter::slot_count/has_slot to support the check. Covered at the unit tier (GCounter, GridStateSnapshot::merge_tenant_spend_from_origin) and the integration tier (StateBroadcastHandler::receive_item, the real SWIM gossip-ingest entry point). E2E coverage is intentionally skipped: exercising a 256-distinct-origin adversarial scenario against a live cluster is impractical and low-signal, the same class of exception already accepted for MAX_TRACKED_TENANTS itself (unit-tier only, no integration/E2E coverage). This closes the site-slot-cap half of grid#52 (proposal item 2). The budget-epoch/window-reset half (proposal item 1) is deliberately out of scope here -- it needs CRD schema changes and cross-site epoch fencing to avoid a monotonic GCounter re-merge reviving a "reset" total, and is tracked separately in a new follow-up issue. Part of grid#52 (site-slot-cap half only; issue stays open for the epoch-reset half). Signed-off-by: Jordi Gil <jgil@redhat.com>
Contributor
Author
praxis-bot
reviewed
Aug 14, 2026
praxis-bot
left a comment
praxis-bot
left a comment
There was a problem hiding this comment.
PR Review
Summary: Caps distinct origin site-slots per tenant's tenant_spend GCounter to bound memory growth from churning or compromised origins (issue #52, item 2).
Assessment: The cap logic in merge_tenant_spend_from_origin correctly gates on has_slot/slot_count before merging, and the check ordering with the existing MAX_TRACKED_TENANTS guard is sound: a brand-new tenant enters with 0 slots, so no empty GCounter leaks into the map via or_insert_with. The retain_origin call upstream guarantees each merge adds at most one slot, making the pre-check slot_count() >= MAX_TENANT_SPEND_ORIGINS sufficient without a post-merge recheck.
| Severity | Count |
|---|---|
| Critical | 0 |
| Large | 0 |
| Medium | 1 |
Addresses grid#63 review feedback (praxis-bot): six new test bodies in grid_state.rs and state_broadcast.rs had leading comment blocks carrying the test's behavioral rationale, violating the project convention (CLAUDE.md, Test Organization: "No comments in test bodies — use assertion messages"). Folded each comment's context into the relevant assertion's message instead of dropping it outright, since the rationale (why the boundary/independence/still-merges behavior matters) wasn't otherwise captured. Signed-off-by: Jordi Gil <jgil@redhat.com>
nerdalert
approved these changes
Aug 16, 2026
nerdalert
pushed a commit
that referenced
this pull request
Aug 16, 2026
…st (#67) resolve-source's "Verify PR merge checkout" step compares the checked-out merge commit's parents against a live-resolved base_sha (fixed in #62), but the checked-out commit itself still came from GitHub's refs/pull/<n>/merge -- a commit GitHub computes and caches asynchronously. Per GitHub's own changelog ("Changes to test merge commit generation for pull requests", 2026-02-19), that ref is only regenerated on a push to the PR branch, a merge-base change, or a 12h max-age timer, and viewing the PR no longer forces a refresh. Live reproduction against PR #63 right after #62 merged: the ref stayed stale for 49+ minutes, including after an explicit mergeable recheck, which would fail "Verify PR merge checkout" with a "base changed" error for a transient, unrelated-to-the-PR reason (see grid#66). Stop depending on that ref. resolve-source already resolves head_sha (PR API) and base_sha (live refs/heads/main) as pinned commit SHAs -- construct the merge locally from those two SHAs with `git merge --no-ff` instead, in both resolve-source (to fail fast on real conflicts before the 120-minute glb-e2e job) and glb-e2e's checkout (to get the actual tested tree). Pinning author/committer identity and date makes the merge commit fully deterministic, so both jobs independently arrive at the identical SHA despite running on separate runners with no shared git state -- verified live: two independent fresh-clone reconstructions of PR #63's merge (base 7ebabdb.., head 50720c9..) produced the exact same commit SHA (6df4a70a..). fetch-depth changes from 2 to 0 (full clone) for the shared checkout step, since `git merge` needs enough history to find the true merge-base between base_sha and head_sha, which a shallow clone cannot guarantee for a PR that is many commits behind main. Fixes grid#66 Signed-off-by: Jordi Gil <jgil@redhat.com>
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
3 participants
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.
What
Bounds the number of distinct origin site-slots a single tenant's
tenant_spendGCountercan accumulate, closing the site-slot-cap half of #52 (proposal item 2).Why
#47's fix (correctly) stopped pruningtenant_spendon SWIM membership eviction, but that traded away the growth bound the buggy behavior accidentally provided. Nothing today removes a site's slot from a tenant's counter once recorded, so a compromised or churning origin repeatedly claiming new site identities could grow a single tenant's counter without bound.MAX_TRACKED_TENANTSalready bounds distincttenant_idkeys — this is the independent, previously-missing bound on distinct origins within one tenant's counter that #52 called out.How
GCounter::slot_count/has_slot— new introspection needed to check the cap.GridStateSnapshot::merge_tenant_spend_from_origin: newMAX_TENANT_SPEND_ORIGINS = 256bound, enforced the same wayMAX_TRACKED_TENANTSalready is — a brand-new origin is refused once a tenant's counter is at capacity; already-tracked origins keep accepting updates.Testing (pyramid invariant)
crdt):GCounterslot_count/has_slot behavior, plus 4 newgrid_statetests pinning the cap's refuse/admit/boundary/independence behavior.swim): 2 new tests exercise the cap through the real SWIM gossip-ingest entry point (StateBroadcastHandler::receive_item), not just the internal merge function directly.MAX_TRACKED_TENANTSitself has unit-tier coverage only, no integration/E2E.All of
cargo fmt,cargo clippy -D warnings,cargo doc(deny-level rustdoc link checks),cargo machete, and this repo'sai-slop-lintpass clean.Scope
This PR intentionally covers only proposal item 2 from #52 (the site-slot cap). Proposal item 1 (the budget-epoch/window reset) needs CRD schema changes plus cross-site epoch-fencing design to avoid a monotonic
GCounterre-merge reviving a "reset" total — that's real additional design surface, so it's tracked as its own follow-up issue rather than bundled here. #52 stays open until that lands.Closes half of #52 (item 2); does not close the issue.