Repository navigation
Fix batching random durations - #76
Merged
Merged
Conversation
A case enters a batched task's waiting list when its previous task is computed, which happens ahead of time, so the list holds cases that reach the task only later and is not ordered by time. - After a batch fires, remove from the waiting list exactly the cases the batch takes, not the first N in insertion order. Before, a case could run twice (crash) while another one was lost. - The firing rule and the batch size only look at cases that have reached the batched task by now, in the order they reached it, so they agree on which cases are waiting. - Use total_seconds() instead of .seconds for waiting times; .seconds wraps negative differences and drops whole days. Cases that haven't reached the batched task are no longer swept into an earlier batch, so the last cases of a run are now fired by the end of the run, which needs two more fixes: - At the end of the run, waiting cases whose last gap is below the low boundary get a firing time instead of being lost, and the waiting list is sorted by the time cases reached the task. - A single waiting case under large_wt waits up to the high boundary instead of firing on the low one.
With a ready_wt rule, the batch size has always made a single waiting case wait for a second one up to the high boundary, but the firing rule said it should fire once the low boundary passed. The two disagreed, nothing fired, and the "batch size ... 0" warning was printed, also with fixed task durations. The low-boundary part of the rule now stays false for a single waiting case, so it agrees with the batch size.
The firing rule boundaries are whole seconds (e.g. "> 7200" becomes a low boundary of 7201). total_seconds() gives fractions, so a wait of 7200.6 s passed "> 7200" in the firing rule while the batch size saw it below 7201 and returned 0, printing the "batch size ... 0" warning. timedelta.seconds rounded down but dropped whole days. whole_seconds() rounds down like .seconds and keeps the days; it is used for the waiting times compared with the boundaries. The batching tests now also put the global random generators' states back after each test, so later tests draw the same values as without them.
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.
Fix batching with random task durations
Cases join a batched task's waiting list ahead of time, with the time they will reach the task, so with random durations the list is out of order and holds future times. Batching treated it as ordered, which caused "batch size 0" warnings, crashes (
not enough values to unpack), and lost cases.Fixes
.secondsreplaced by whole seconds, with days kept.large_wtcase firing too early (this also fixes a flaky test).Testing: new
test_batching_random_durations.pyand the full suite is green.