jenkinsci / jenkinsci/durable-task-plugin
Improving CauseOfBlockage during queue maintenance with cache
- Dominant language
- Java
- Stars
- 30
- Forks
- 101
- PR merge metrics
- No merged PRs in 30d
Description
### 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` https://github.com/jenkinsci/durable-task-plugin/blob/0059693971a75e97fde6f453624b0e2e85eeab48/src/main/java/org/jenkinsci/plugins/durabletask/executors/ContinuedTask.java#L60 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_
Contributor guide
Assessment
This issue has not been assessed yet.