Conversation
|
Verified that @tjlee0909 has signed the CLA. Thanks for the pull request! |
There was a problem hiding this comment.
I thought that breaking up the conditionals was much easier to read than all of the 'ands' and 'nots' chained together.
tjlee0909
force-pushed
the
done_allocate_more_executors_to_build_than_subjobs
branch
from
958d9d2 to
1e27531
Compare
|
I posted a comment about this in #273 , but to reiterate: I'm going for low-hanging fruit here with the somewhat common manifestation of this bug where the max_executors is set to much higher than the actual number of subjobs calculated by the atomizer. |
There was a problem hiding this comment.
Note to self: maybe should compare to self._build._unstarted_subjobs.qsize() instead.
There was a problem hiding this comment.
I think I actually prefer checking the total number of subjobs. What were you thinking would be the advantage of checking _unstarted_subjobs.qsize()?
There was a problem hiding this comment.
In the scenario where the slaves being allocated are trickling in (being released from other builds onto this one), then the length of all subjobs is an overestimate, whereas the unstarted_subjobs.qsize() is more of an exact fit.
It's really not a big deal though. I'm happy with leaving it as is and merging it.
There was a problem hiding this comment.
Yeah agreed / not a big deal. 🤘
|
Hallelujah! 👍 |
…d_than_subjobs Don't allocate more executors to a build than total subjobs
3 participants
This addresses #273