Correctly calculate new manager real capacity #3823
Conversation
The inner else condition was never utilized before, as the value of `real_capacity` never changed. Calculate the value correctly by including the tasks that moved.
khk-globus
force-pushed
the
correctly_calculate_real_capacity
branch
from
086063f to
0acdafb
Compare
| @@ -481,7 +481,7 @@ def process_tasks_to_send(self, interesting_managers: Set[bytes]) -> None: | |||
| m['idle_since'] = None | |||
| logger.debug("Sent tasks: %s to manager %r", tids, manager_id) | |||
| # recompute real_capacity after sending tasks | |||
| real_capacity = m['max_capacity'] - tasks_inflight | |||
| real_capacity -= task_count | |||
There was a problem hiding this comment.
thinking this through to understand it myelf.. and if rewriting this a bit more...
the point of this line is to feed into the if statement below, and nowhere else. the if statement is asking "did we schedule as many tasks as we wanted to (did will fill the original real_capacity?) or do we still have capacity (in which case the manager remains interesting). the actual numeric value of real_capacity is irrelevant here beyond whether it is positive or not.
This assertion should hold, though:
assert real_capacity == m['max_capacity'] - len(m['tasks']) which is the calculation that happens earlier at line 471.
There was a problem hiding this comment.
... and then maybe the line 486 debug message would be phrased as "still has free capacity"
Description
The inner else condition was never utilized before, as the value of
real_capacitynever changed. Calculate the value correctly by including the tasks that moved.Changed Behaviour
No user workflows should be changed; managers will be removed from the interesting_managers variable one iteration sooner.
Type of change