Summary
The current shard-cap enforcement in register_shards, split_replace, and split_residual uses a preflight read (current_shard_counts + shard_limit_violation) that is not atomic with the shard-creating CAS transaction. Two concurrent callers under the same tenant can both observe a count below the ceiling and both commit, silently exceeding max_shards_per_tenant / max_total_shards.
Raised in PR #148: #148
Originating review comment: #148 (comment)
Requested by: @ahrav
Affected paths
| Path |
Operation |
backend/run_management.rs |
register_shards (sync + async) |
backend/shard_coordination.rs |
split_replace (sync + async) |
backend/shard_coordination.rs |
split_residual (sync + async) |
| shard-removing paths |
complete_run, cancel_run, fail_run (decrement on removal) |
Design sketch
Sentinel key shape
Introduce two etcd keys:
{ns}/tenants/{tenant_id}/shard_count — per-tenant count (value: little-endian u64)
{ns}/global/shard_count — global count (value: little-endian u64)
CAS compare strategy
In every shard-creating transaction, add etcd Compare conditions that assert the sentinel values are below the respective ceilings before committing shard records. On success, atomically increment both sentinels (via a put of current + delta). On failure (Compare rejects), return ShardLimitExceeded.
Note: etcd does not support arithmetic compare-and-increment natively, so the pattern is: read both sentinels in a preflight get, include compare(version, sentinel_key, current_version) (or compare(value, sentinel_key, current_value)) in the same txn, and write current_value + delta as the new sentinel value. If the txn fails due to the Compare, reload and retry (the existing CAS retry loop handles this).
Shard-removing paths
Decrement both sentinels in the same txn that removes shard records (e.g., terminal run transitions when shard records are garbage-collected, or explicit remove paths). Use a guarded decrement (compare-and-swap) to avoid underflow.
Bootstrap semantics
On first use (sentinel absent), treat a missing sentinel key as count = 0. The first shard-creating txn should include a compare_absent(sentinel_key) branch that writes the initial value, or a separate bootstrap step during connect().
Contention tradeoffs
- The per-tenant sentinel key serializes all concurrent shard-creating writes for the same tenant. Under low concurrency this is fine; under high concurrency the CAS retry loop absorbs contention.
- The global sentinel is a hot key if many tenants create shards simultaneously. Consider whether the global ceiling is strict (sentinel) or advisory (preflight-only) based on operational requirements.
Out of scope for this issue
Summary
The current shard-cap enforcement in
register_shards,split_replace, andsplit_residualuses a preflight read (current_shard_counts+shard_limit_violation) that is not atomic with the shard-creating CAS transaction. Two concurrent callers under the same tenant can both observe a count below the ceiling and both commit, silently exceedingmax_shards_per_tenant/max_total_shards.Raised in PR #148: #148
Originating review comment: #148 (comment)
Requested by: @ahrav
Affected paths
backend/run_management.rsregister_shards(sync + async)backend/shard_coordination.rssplit_replace(sync + async)backend/shard_coordination.rssplit_residual(sync + async)complete_run,cancel_run,fail_run(decrement on removal)Design sketch
Sentinel key shape
Introduce two etcd keys:
{ns}/tenants/{tenant_id}/shard_count— per-tenant count (value: little-endian u64){ns}/global/shard_count— global count (value: little-endian u64)CAS compare strategy
In every shard-creating transaction, add etcd
Compareconditions that assert the sentinel values are below the respective ceilings before committing shard records. On success, atomically increment both sentinels (via a put ofcurrent + delta). On failure (Compare rejects), returnShardLimitExceeded.Shard-removing paths
Decrement both sentinels in the same txn that removes shard records (e.g., terminal run transitions when shard records are garbage-collected, or explicit remove paths). Use a guarded decrement (compare-and-swap) to avoid underflow.
Bootstrap semantics
On first use (sentinel absent), treat a missing sentinel key as count = 0. The first shard-creating txn should include a
compare_absent(sentinel_key)branch that writes the initial value, or a separate bootstrap step duringconnect().Contention tradeoffs
Out of scope for this issue
AsyncShardClaiming/AsyncCoordinationFacade) — tracked in issue Add AsyncShardClaiming / AsyncCoordinationFacade to complete the async coordination surface #149.