Repository navigation
Fix handling of cancelled tickets. - #40588
Conversation
tvalentyn
commented
Oct 14, 2026
- Fixes an out-of-bounds error when all tickets in the wait queue are cancelled.
- drive-by: don't count cancelled tickets towards model demand signal.
|
R: @AMOOOMA |
There was a problem hiding this comment.
📋 Review Summary
This PR fixes an IndexError in ModelManager._wake_next_in_queue when all remaining tickets in _wait_queue have been cancelled, and excludes cancelled tickets from the demand calculation in _evict_to_make_space. The changes are clean, concise, and well-covered by new unit tests.
🔍 General Feedback
- Moving the
if self._wait_queue:check after draining cancelled tickets at the head of the heap properly prevents out-of-bounds access when the queue becomes empty. - Filtering out
self._cancelled_ticketswhen computingdemand_mapensures timed-out requests do not artificially protect idle models from eviction.
💡 Tip: Comment /gemini review on this pull request to trigger a new Gemini code review.
|
Stopping reviewer notifications for this pull request: review requested by someone other than the bot, ceding control. If you'd like to restart, comment |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #40588 +/- ##
=============================================
+ Coverage 56.16% 79.93% +23.76%
=============================================
Files 1124 488 -636
Lines 178336 82348 -95988
Branches 1489 0 -1489
=============================================
- Hits 100170 65823 -34347
+ Misses 75645 16525 -59120
+ Partials 2521 0 -2521
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
AMOOOMA
left a comment
There was a problem hiding this comment.
Thanks! This looks good to me, good catch on the subtle clean up logic.