What feature do you want to see added?
Hey, I'd love to see ContinuedTask.CauseOfBlockage cached to speed up queue maintenance. We have seen some slowness in terminating agents after their jobs had finished, and looking at some thread dumps, found out that the queue maintenance was very slow and hangs around GetCauseOfBlockage.
"AtmostOneTaskExecutor[Periodic Jenkins queue maintenance] [#222258]" #626708 [564800] daemon prio=5 os_prio=0 cpu=148915.38ms elapsed=152.48s tid=0x000073912c0196a0 nid=564800 runnable [0x00007388bdbfd000]
java.lang.Thread.State: RUNNABLE
at org.jenkinsci.plugins.durabletask.executors.ContinuedTask$Scheduler.canTake(ContinuedTask.java:65)
at hudson.model.Queue$JobOffer.getCauseOfBlockage(Queue.java:285)
at hudson.model.Queue.maintain(Queue.java:1774)
at hudson.model.Queue$1.call(Queue.java:339)
at hudson.model.Queue$1.call(Queue.java:336)
at jenkins.util.AtmostOneTaskExecutor.lambda$maybeRun$0(AtmostOneTaskExecutor.java:107)
at jenkins.util.AtmostOneTaskExecutor$$Lambda/0x0000000080a73d48.call(Unknown Source)
at jenkins.security.ImpersonatingExecutorService.lambda$wrap$1(ImpersonatingExecutorService.java:75)
at jenkins.security.ImpersonatingExecutorService$$Lambda/0x0000000080a7a000.call(Unknown Source)
at java.util.concurrent.FutureTask.run(java.base@21.0.10/Unknown Source)
at hudson.remoting.AtmostOneThreadExecutor$Worker.run(AtmostOneThreadExecutor.java:121)
at java.lang.Thread.runWith(java.base@21.0.10/Unknown Source)
at java.lang.Thread.run(java.base@21.0.10/Unknown Source)
The queue maintenance outer loop iterates through each buildable, and for each buildable iterates through idle executors: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/Queue.java#L1768
Then inside that loop, it runs getCauseOfBlockage
|
@Override public CauseOfBlockage canTake(Node node, Queue.BuildableItem item) { |
which does another buildable traverse so each queue.maintain ends up being
(buildables)^2 * idle executors.
ContinuedTask checks if the executor can take the buildable by looping through all other buildables to see if any continued task is tied to the node. Instead of looking through all buildables, we could store all continued task in a map keyed by id and (label, task, etc).
The first iteration requires an atomic initialization to walk the buildables list and map them to their task, label, etc. Then every time we run canTake(), we walk the length of the continued tasks instead of all buildables. On leaving or entering buildable, we evict/add to the cache.
I considered caching by node -> continued task but that makes cache eviction tedious. I think it's more important to remove the quadratic loop. Would welcome feedback if anyone has run into this before. We have an internal patch that we are testing, would be happy to post for feedback
Upstream changes
No response
Are you interested in contributing this feature?
No response
What feature do you want to see added?
Hey, I'd love to see
ContinuedTask.CauseOfBlockagecached to speed up queue maintenance. We have seen some slowness in terminating agents after their jobs had finished, and looking at some thread dumps, found out that the queue maintenance was very slow and hangs aroundGetCauseOfBlockage.The queue maintenance outer loop iterates through each buildable, and for each buildable iterates through idle executors: https://github.com/jenkinsci/jenkins/blob/master/core/src/main/java/hudson/model/Queue.java#L1768
Then inside that loop, it runs
getCauseOfBlockagedurable-task-plugin/src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java
Line 60 in 0059693
(buildables)^2 * idle executors.ContinuedTaskchecks if the executor can take the buildable by looping through all other buildables to see if any continued task is tied to the node. Instead of looking through all buildables, we could store all continued task in a map keyed byidand(label, task, etc).The first iteration requires an atomic initialization to walk the buildables list and map them to their task, label, etc. Then every time we run
canTake(), we walk the length of thecontinued tasksinstead of all buildables. On leaving or entering buildable, we evict/add to the cache.I considered caching by node -> continued task but that makes cache eviction tedious. I think it's more important to remove the quadratic loop. Would welcome feedback if anyone has run into this before. We have an internal patch that we are testing, would be happy to post for feedback
Upstream changes
No response
Are you interested in contributing this feature?
No response