abderrahim commented on code in PR #2005:
URL: https://github.com/apache/buildstream/pull/2005#discussion_r2094049650
##########
src/buildstream/_scheduler/scheduler.py:
##########
@@ -371,7 +371,7 @@ def _sched_queue_jobs(self):
# Pull elements forward through queues
elements = []
- for queue in queues:
+ for queue in self.queues:
Review Comment:
> The first part of the function's job is to select the queues on which to
process in the latter half of the function, that seems to make sense. And
except for this line you've changed, the rest of the function does only operate
on the selected queues.
Correct, and that's the root cause of the issue I'm trying to fix.
There are two operations in this function ("pull elements forward through
queues", and "kickoff whatever processes can be processed at this time") and
they shouldn't operate on the same list of queues.
Let's consider a simple example where we have three queues: fetch, build and
push. Once we receive the quit signal, we only want to:
* pull elements forward from build to push
* kickoff new jobs in push
What the current code does is:
* pull elements forward from build to push
* kickoff new jobs in build (<- not desired, and is the issue I'm trying to
fix)
* kickoff new jobs in push
What the new code does is:
* pull elements forward from fetch to build (<- undesired but harmless, this
is the issue you're pointing)
* pull elements through from build to push
* kickoff new jobs in push
I hope this explains it. So while this line is indeed "dubious", it needs to
be different from the other one. And since it's harmless to pull jobs into next
queues even when asked to quit, I think it's fine to do that unconditionally.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
