Skip to content

fix: Don't drop work queued by the last active table in shuffle-queue - #2633

Merged
kodiakhq[bot] merged 4 commits into
mainfrom
claude/mystifying-diffie-a5f254
Oct 15, 2026
Merged

kodiakhq[bot] merged 4 commits into
mainfrom
claude/mystifying-diffie-a5f254

Conversation

@disq

@disq disq commented Oct 15, 2026 •

Copy link
Copy Markdown
Member

Summary

The shuffle-queue dispatcher now gets work from a workQueue that pops and checks for idle under one lock, and wakes on Push, Done or context cancel.

@disq
disq requested a review from a team as a code owner October 15, 2026 10:58
@disq
disq requested a review from mnorbury October 15, 2026 10:58

@erezrokah erezrokah left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, no blockers. The buffered workDone signal cannot lose a Done, and the second Pop catches relations that the last active table queued.

Follow-ups, not blockers:

  • The dispatcher wakes only on Done, not on Push. Relations from a long-running table wait until a worker finishes. A Push signal also removes the need for the second Pop.
  • isStarted is not necessary now. It can also make dispatch block forever if the queue starts empty.
  • Wait does not use ctx. Consider select on ctx.Done().

@disq

disq commented Oct 15, 2026

Copy link
Copy Markdown
Member Author

@erezrokah thanks, all three are handled in 940549b. activeWorkSignal is replaced by a workQueue that owns the queue and the in-progress count under one lock:

  • Wake on Push: Push wakes the dispatcher, so relations no longer wait for a Done. The second Pop is gone, because Next pops and checks for idle under the same lock.
  • isStarted: removed. Next returns nil at once when the queue starts empty.
  • ctx: Next selects on ctx.Done() and returns nil when the context is cancelled.

The tests in work_queue_test.go use testing/synctest, so they do not depend on timing.

@disq disq added the automerge label Oct 15, 2026
@kodiakhq
kodiakhq Bot merged commit 223018b into main Oct 15, 2026
10 checks passed
@kodiakhq
kodiakhq Bot deleted the claude/mystifying-diffie-a5f254 branch October 15, 2026 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants