qscheduler could return a task from AssignTasks while it is still awaiting cancellation on its previous worker |
|||
Issue descriptionper a thread with nodir@ 1) AssignTasks on QS is called, and QS decides to cancel task A. 2) Before GetCancellations can be called and the task can be actually cancelled, a new worker becomes idle and calls AssignTasks on QS; QS decides to give it task A. Swarming will be unable to reap that task to a new worker, because it's swarming state is still running on the original worker. I think this can be addressed by adding an intermediate "cancelling" state withing QS. Task A would sit in cancelling state in QS until either NotifyRequest(Task A, Idle) or NotifyRequest(New Task, Original Worker) call was received.
,
Nov 7
The following revision refers to this bug: https://chromium.googlesource.com/infra/infra/+/7652189e6f7f55d5b170a882dd80353355966727 commit 7652189e6f7f55d5b170a882dd80353355966727 Author: Aviv Keshet <akeshet@chromium.org> Date: Wed Nov 07 23:52:28 2018 qscheduler: don't reenqueue cancelled requests until ACKed The previous scheduler behavior exposed a race condition, in which it would be possible for the scheduler to return a task from AssignTasks that was not idle (because the scheduler believed it to be cancelled and reenqueued instantly, whereas the actual cancellation and reenqueue process is multi-step). BUG= chromium:900664 TEST=Existing tests modified prior to fix. Change-Id: If0326c64588ead9ae5e7a77087d4bbdc21087037 Reviewed-on: https://chromium-review.googlesource.com/c/1321168 Reviewed-by: Andrii Shyshkalov <tandrii@chromium.org> Commit-Queue: Aviv Keshet <akeshet@chromium.org> Cr-Commit-Position: refs/heads/master@{#18852} [modify] https://crrev.com/7652189e6f7f55d5b170a882dd80353355966727/go/src/infra/qscheduler/qslib/reconciler/reconciler_test.go [modify] https://crrev.com/7652189e6f7f55d5b170a882dd80353355966727/go/src/infra/qscheduler/qslib/scheduler/state.go [modify] https://crrev.com/7652189e6f7f55d5b170a882dd80353355966727/go/src/infra/qscheduler/qslib/scheduler/state_test.go
,
Nov 27
,
Dec 4
|
|||
►
Sign in to add a comment |
|||
Comment 1 by akes...@chromium.org
, Nov 2