Do not acquire lock when decreasing task count.#4277
Do not acquire lock when decreasing task count.#4277kcmannem merged 1 commit intoconcourse:masterfrom
Conversation
The tasks count decreasing (used in the limit-active-tasks strategy) can be executed concourrently as the DB transaction will make sure that the end value is consistent. Trying to acquire a lock when decreasing the tasks count in a system with several flying task can easily result in a race to acquire lock and eventually even a "de facto" dead lock. Signed-off-by: Alessandro Degano <[email protected]>
aledegano
force-pushed
the
no_locks_decrease_tasks
branch
from
4cd63f6 to
2047500
Compare
|
@aledeganopix4d interesting! I apologize for not answering your question about this in another thread earlier - I filed it away to respond to later and it completely slipped my mind. We did not consider potential lock contention. The reason we added the lock was that in the case that between reading the number of tasks and actually selecting a worker, the number of tasks on a given worker might decrease. This is not a huge deal since in the next loop iteration the worker would be selected, but we were aiming for correctness. If it truly is causing this much contention then I agree with you, we can just rely on DB locking as the failure case is not that much of a problem. |
|
@ddadlani sorry for nagging: do you think we can merge this one? |
jamieklassen
added
the
release/undocumented
#4118 #4148 #4208 #4277 #4142 #4221 #4293 Signed-off-by: James Thomson <[email protected]> Co-authored-by: Jamie Klassen <[email protected]>
#4118 #4148 #4208 #4277 #4142 #4221 #4293 Signed-off-by: James Thomson <[email protected]> Co-authored-by: Jamie Klassen <[email protected]>
5 participants
The tasks count decreasing (used in the limit-active-tasks strategy) can be
executed concourrently as the DB transaction will make sure that the end
value is consistent.
Trying to acquire a lock when decreasing the tasks count in a system with
several flying task can easily result in a race to acquire lock and eventually
even a "de facto" dead lock.
We observed this issue on our Production CI where it eventually all just hanged, what happens is the following:
@deniseyu
@ddadlani