Refactor Snake into a queue 2/3 - #925
Draft
bgwines wants to merge 5 commits into
Draft
Conversation
bgwines
force-pushed
the
bwines/snake-22-snake-queue
branch
4 times, most recently
from
August 23, 2026 20:13
7609baa to
484d937
Compare
bgwines
force-pushed
the
bwines/snake-22-snake-cleanup
branch
from
August 23, 2026 21:11
2a4ae73 to
defb029
Compare
bgwines
force-pushed
the
bwines/snake-22-snake-queue
branch
from
August 23, 2026 21:11
5901915 to
aaed370
Compare
AI assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
AI assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
AI assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
AI assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
bgwines
force-pushed
the
bwines/snake-22-snake-cleanup
branch
from
August 23, 2026 21:21
defb029 to
d43468e
Compare
bgwines
force-pushed
the
bwines/snake-22-snake-queue
branch
from
August 23, 2026 21:21
aaed370 to
c5d8643
Compare
bgwines
commented
Aug 24, 2026
| // result channel, and updates bookkeeping. Use with care: this bypasses the | ||
| // health-state transitions in peek/dequeue, so callers are responsible for | ||
| // updating dropping state if appropriate. | ||
| func (q *CoDelQueue[T]) lockedPopElem(elem *list.Element, err error) *Request[T] { |
Author
There was a problem hiding this comment.
this function existed publicly only for the cancel-after-grant race. Snake doesn't own granting anymore, so it isn't needed. There were one or two internal callers, dequeue and peek. Dequeue still gets all of the functionality it needs, and peek used it for cleanup of canceled requests, but cancelation is different now; entries are synchronously removed upon cancel, so we don't need the clean-up (see PR description)
AI assisted with development. Every line of code was either written by or carefully reviewed by me :) Signed-off-by: Brett Wines <bwines@slack-corp.com>
bgwines
force-pushed
the
bwines/snake-22-snake-queue
branch
from
August 24, 2026 02:03
c5d8643 to
4beb66b
Compare
bgwines
commented
Aug 24, 2026
Comment on lines
-306
to
-315
| if r.isDroppable() { | ||
| q.droppable.remove(r) | ||
| q.droppableLen-- | ||
| if q.droppableLen == 0 { | ||
| q.dropping = false | ||
| } | ||
| } | ||
| q.lockedAdvanceFirstWaiting(r.codelqElem) | ||
| q.queue.Remove(r.codelqElem) | ||
| r.codelqElem = nil |
Author
There was a problem hiding this comment.
this behavior wasn't removed; it just lives in lockedRemove()
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
1 participant
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.
Background / Why?
Snake now arbitrates queued work while the caller owns execution capacity, so its resident-grant lifecycle adds state and synchronization without serving the queue contract. Narrowing the implementation to enqueue, dequeue, cancellation, and drop reporting makes the load-shedding component reusable by connection pools and preserves the CoDel behavior that matters.
Cancellation is now caller-owned as well. Cancelling a context does not terminate its goroutine or immediately mark its queue entry; the waiting caller observes cancellation, takes the parent mutex, and calls
Snake.Cancel. A dequeue may win during that transient window. If cancellation wins, it removes the entry; if dequeue wins, cancellation observes the request’s terminal state and the caller completes the committed handoff. Under normal execution, this guarantees that cancellation cannot leave a stale resident entry, soPeekno longer needs to clean up cancelled queue heads.Testing
New unit tests covering queue dequeue, cancellation, valve promotion, and disabled shedding.
AI assisted with development. Every line of code was either written by or carefully reviewed by me :)