Reject malformed live budget counters - #62
Closed
albertovincenzi wants to merge 3 commits into
Closed
Conversation
The two halves of a counter row cannot fail the same way, and treating them alike put an unbounded outage on the admission path. `charge` is the hot path, and a failed charge is NOT a refusal: the relay reads it as "the batch did not happen" and releases the claim, so the identical batch comes back and fails again. A single row with a missing or unrenderable expiry therefore stopped every path on that counter for ever, at twice the kv write volume, admitting nothing and healing never. The row is reachable — an operator `put` without a TTL is exactly what this branch's own test plants. An expiry costs the PARK DEADLINE and nothing else, and `None` there has always meant "retry now" rather than "wait for ever". So it degrades, loudly, and the charge is still decided by the counter. A VALUE that is not an integer keeps failing closed: there is nothing to reason about, and the read-side lie this branch exists to remove is precisely that one being reported as zero. The live test now plants that case for the 502 assertions and proves the expiryless counter still admits. Claude-Session: https://claude.ai/code/session_012K8u7BEJyd6nDNMCQAgH3z
Contributor
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
2 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.
Problem
A present Queen KV budget row is currently decoded with defaults: a non-integer or missing value becomes 0, and a missing or malformed expiresAt becomes "retry now". Unexpected and duplicate keys are accepted too.
That makes corrupt or version-skewed live state look healthy. Operator APIs can show unused budget, ETA can become too optimistic, and the relay can repeatedly retry a type-refused counter with no real window deadline.
There is a second integrity edge: the batched charge may have applied some increments before its trailing read reveals malformed state. Returning only an error without compensation would leak those known charges.
Fix
Before / after
Before: corrupt present state could be reported as value 0 with no wait, or could drive a rapid refusal loop.
After: reads fail explicitly (HTTP 502 through #59), relay work is released for later redelivery, and any provably applied increments are guarded and refunded.
Verification
Review / dependency
This PR is intentionally based on #59 so read-side Queen errors already have one consistent HTTP 502 contract. Review commit 031beb4 as this complete fix. It is independent of #61 and can be rebased after #59 lands.