fix(gateway): stop leaking quota reservations and rule keys - #369
Open
ecv wants to merge 1 commit into
Open
Conversation
Engine's failure paths leaked in three ways, and KernelDatapath lost track of a partially-applied rule's keys. applyRuleLocked reserved quota, then returned on a Datapath.ApplyRule error without releasing it. Reconcile only adds a key to e.active on success, so removeRuleLocked never ran for it and the reservation was stranded for the life of the process. It is now released on that path. removeRuleLocked returned on a Datapath.RemoveRule error before reaching quota.Release, so a rule the caller had already withdrawn kept its reservation. The release now happens either way and the datapath error is still returned, joined with the release error when both fail. NodeQuotaEnforcer.Release is a no-op for an unreserved key, so the caller's retry of a failed teardown stays correct. Stop's guard read `err != nil && firstErr == nil`, so once firstErr was set every later failed teardown fell through to the delete and was dropped from the active set anyway. Failure handling is now symmetric: a key whose teardown failed stays in e.active regardless of whether an earlier key already failed. Stop still returns the first error. KernelDatapath.ApplyRule's Register loop returned on the first error while ruleKeysByName was only assigned after the loop, so keys already written to rule_table were untracked and RemoveRule could not find them. The successfully-registered subset is now recorded before returning, alongside the keys the rule already owned, since the prune has not run at that point. Adds engine tests for quota release after a failed apply, the active set and quota state after a failed removal, and a Stop where both teardowns fail. Related to #359 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
ecv
force-pushed
the
fix/gateway-quota-teardown-leaks
branch
from
August 13, 2026 01:23
16a2b2d to
d426b22
Compare
ecv
marked this pull request as ready for review
August 13, 2026 03:10
privateip
approved these changes
Aug 13, 2026
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.
Summary
The gateway engine reserves per-tenant capacity before it programs a rule, and releases it when the rule goes away. Three failure paths did not hold up their end, and a fourth lost track of what had been written.
A rule that passed the quota check and then failed to program kept its reservation for the life of the process. It never entered the active set, so nothing was ever going to release it. Delete that rule and the capacity was gone until a restart, on a node whose table was mostly empty.
Teardown released capacity only when datapath removal succeeded. Now it releases either way, and still surfaces the datapath error rather than swallowing it.
Shutdown handled the second and later failures differently from the first: they were dropped from the active set despite having failed, so the engine reported state gone that the datapath still held. Failures are now treated the same regardless of order.
The fourth is a layer down. Programming a rule with several addresses writes them one at a time, and the bookkeeping teardown relies on was only written after all of them succeeded. A failure partway through left earlier writes untracked. Those are now recorded as they land.
Three tests cover what the suite could not see before: capacity released after a failed apply, active-set and capacity state after a failed teardown, and a shutdown where both rules fail to tear down.
Known remaining case
The prune step in the same method has the same shape. If unregistering a dropped address fails, keys registered earlier in that pass go untracked. The orphan sweep collects them, exactly as it did before this change. Left alone deliberately to keep the diff to the reported defects.
Test plan
task linttask buildtask test:unit, including the three new casestask test:e2eCI is the gate, since this machine cannot build the package.
Related to #359