You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
capsule_server::counter::InMemoryCounters holds all twelve CounterKey partitions behind a single Mutex<Partitions>. Give each partition its own lock, or replace the map with a sharded/concurrent structure, so a flood against one key kind does not serialise hit/peek/reset for the others.
Why this is filed rather than fixed
Raised during review round 4 of #459 and deliberately deferred there.
Decision 22 on that PR partitioned the ceiling per CounterKey variant, so admission is independent: one partition's occupancy is invisible to another's ceiling, and filling DropLink cannot deny a first key to ShareLink or OidcAuthorize. Latency is not independent, because every partition shares one lock.
That is a real residual and not a practical denial at the current sizes:
the critical section holds no .await — it is a purge, a BTreeMap lookup and at most one insert;
the work is O(log n) over at most 20 000 entries (ceilings::DROP_LINK);
the flood that would contend for it is itself bounded by the ceiling it is filling.
So it is contention, not denial. It is worth recording because the module doc claims "the windows one surface holds are not the windows another surface is denied", and that sentence is about admission — it should not be read as a latency guarantee. The doc now says so explicitly.
Raising any ceilings::* constant materially raises the O(log n) work under the shared lock.
A future CounterKey whose partition is both hot and caller-influenced would make the contention easier to reach than it is today.
Suggested shape
Partitions becomes BTreeMap<&'static str, Mutex<BTreeMap<CounterKey, Window>>> built once from the known variant names, so the outer map is read-only after construction and needs no lock at all. That keeps CounterKey::as_str as the partition key and leaves CounterStore's contract untouched.
Whatever the shape, the atomicity CounterStore::hit promises — charge and decide as one operation — must survive: it is the property the whole port exists for, and a per-partition lock preserves it because a key never spans two partitions.
Acceptance
hit on one CounterKey variant does not block hit on another.
The existing counter::tests suite passes unchanged, including no_sequence_of_calls_admits_more_than_the_budget and flooding_one_surface_does_not_deny_a_fresh_key_to_another.
The module docs' "one lock" section is updated or removed.
What
capsule_server::counter::InMemoryCountersholds all twelveCounterKeypartitions behind a singleMutex<Partitions>. Give each partition its own lock, or replace the map with a sharded/concurrent structure, so a flood against one key kind does not serialisehit/peek/resetfor the others.Why this is filed rather than fixed
Raised during review round 4 of #459 and deliberately deferred there.
Decision 22 on that PR partitioned the ceiling per
CounterKeyvariant, so admission is independent: one partition's occupancy is invisible to another's ceiling, and fillingDropLinkcannot deny a first key toShareLinkorOidcAuthorize. Latency is not independent, because every partition shares one lock.That is a real residual and not a practical denial at the current sizes:
.await— it is a purge, aBTreeMaplookup and at most one insert;O(log n)over at most 20 000 entries (ceilings::DROP_LINK);So it is contention, not denial. It is worth recording because the module doc claims "the windows one surface holds are not the windows another surface is denied", and that sentence is about admission — it should not be read as a latency guarantee. The doc now says so explicitly.
When this matters
ceilings::*constant materially raises theO(log n)work under the shared lock.CounterKeywhose partition is both hot and caller-influenced would make the contention easier to reach than it is today.Suggested shape
PartitionsbecomesBTreeMap<&'static str, Mutex<BTreeMap<CounterKey, Window>>>built once from the known variant names, so the outer map is read-only after construction and needs no lock at all. That keepsCounterKey::as_stras the partition key and leavesCounterStore's contract untouched.Whatever the shape, the atomicity
CounterStore::hitpromises — charge and decide as one operation — must survive: it is the property the whole port exists for, and a per-partition lock preserves it because a key never spans two partitions.Acceptance
hiton oneCounterKeyvariant does not blockhiton another.counter::testssuite passes unchanged, includingno_sequence_of_calls_admits_more_than_the_budgetandflooding_one_surface_does_not_deny_a_fresh_key_to_another.Refs #459.