Skip to content

fix(weave): don't stall flush() on unpaired eager calls under calls_complete - #7216

Merged
gtarpenning merged 3 commits into
masterfrom
gtarpenning/wb-34557-flush-eager-unpaired
Jun 15, 2026
Merged

fix(weave): don't stall flush() on unpaired eager calls under calls_complete#7216
gtarpenning merged 3 commits into
masterfrom
gtarpenning/wb-34557-flush-eager-unpaired

Conversation

@gtarpenning

Copy link
Copy Markdown
Member

Summary

  • A mid-eval client.flush() cleared _eager_call_ids, so the later eager call-end no longer matched and parked in _pending_ends as a never-pairing orphan; the next flush then waited the full 5-min FLUSH_TIMEOUT_SECONDS on it.
  • Keep eager tracking across a flush, exclude eager-unpaired ends from the pairing wait, and send them via the eager v2 endpoint right away. Non-eager in-flight work keeps the full pairing budget.
  • WB-34557: https://coreweave.atlassian.net/browse/WB-34557

Testing

new test asserts eager-unpaired flush returns fast while non-eager pending still consumes the full timeout.

…omplete

A mid-eval client.flush() cleared _eager_call_ids, so the eventual eager
call-end no longer matched and parked in _pending_ends as a never-pairing
orphan. The next flush then burned the full FLUSH_TIMEOUT_SECONDS (5 min)
waiting on it. Keep eager tracking across a flush, exclude eager-unpaired
ends from the pairing wait, and send them via the eager v2 endpoint
immediately. Non-eager in-flight work keeps the full pairing budget.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@codecov

codecov Bot commented Jun 15, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

The orphaned-eager-end stall is fixed entirely by not clearing
_eager_call_ids on flush. _handle_end already routes eager ends, so the
_flush_eager_unpaired_ends / _num_non_eager_pending helpers were
unreachable; drop them and tighten the test to assert the no-stall
regression directly.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gtarpenning
gtarpenning marked this pull request as ready for review June 15, 2026 16:52
@gtarpenning
gtarpenning requested a review from a team as a code owner June 15, 2026 16:52

@jtschoonhoven jtschoonhoven Jun 15, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Claude nit:

Lingering assertion in the renamed test is now misleading (239): you dropped "and clears state"/"clears caches" from the name and docstring but kept assert len(processor._eager_call_ids) == 0. That assertion still passes only because both items in that test are non-eager (enqueue(...) with eager_call_start=False), so no eager ids are ever created, not because anything clears them. It now reads like it's verifying clear-on-flush, which is exactly the behavior you removed. Either drop the line or add a comment noting it's just confirming no eager IDs leaked from non-eager calls.

The renamed test no longer verifies clear-on-flush, but kept
assert len(_eager_call_ids) == 0 which only passed because both items
are non-eager. Drop it; the pending-state asserts already cover drain.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
@gtarpenning
gtarpenning enabled auto-merge (squash) June 15, 2026 17:52
@gtarpenning
gtarpenning merged commit 48dac10 into master Jun 15, 2026
107 checks passed
@gtarpenning
gtarpenning deleted the gtarpenning/wb-34557-flush-eager-unpaired branch June 15, 2026 18:04
@github-actions github-actions Bot locked and limited conversation to collaborators Jun 15, 2026
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants