Jenkins.getInstance().getAllItems(ParameterizedJob.class) slow in large installations

[Tomas Bjerre]

It was reported here:
https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/272

That the Generic Webhook Trigger Plugin is slow in large installations.

I "solved" that by adding a cache and having it configurable in the global config here:
https://github.com/jenkinsci/generic-webhook-trigger-plugin/blob/generic-webhook-trigger-2.1.1/src/main/java/org/jenkinsci/plugins/gwt/global/CacheConfig.java

And implemented cache here:
https://github.com/jenkinsci/generic-webhook-trigger-plugin/blob/generic-webhook-trigger-2.1.1/src/main/java/org/jenkinsci/plugins/gwt/jobfinder/JobFinderImpersonater.java

I'd like to get some feedback on this solution.

- Does this kind of cache really belong in the plugin?
- In order to invalidate the cache, the plugin starts a scheduled task. Feels fragile, hard to test and I don't know if it works after a few days of uptime.

- Is there a faster way to Jenkins.getInstance().getAllItems(ParameterizedJob.class); ?

- Users may not understand the effect of enabling this cache https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/302#issuecomment-2051821328

[Winter, Markus]

Instead of refilling the cache every x minutes you could implement an https://javadoc.jenkins.io/hudson/model/listeners/ItemListener.html to react on job changes. This would avoid the problem mentioned in issue 302.

You would need to implement onloaded(), onUpdated(), onCreated() and onDeleted() probably. Maybe also onCopied() and onLocationChanged() depending how you internally manage the data in the cache (instead of a list you would need a map<String, ParameterizedJob> with job fullname as key).

 

When you have your jobs organized in folders then the getAllItems could probably be made faster by loading each folder in its own thread. But with the above approach you would need to do this only once at startup so probably not worth the effort.

 

Regarding https://github.com/jenkinsci/generic-webhook-trigger-plugin/pull/273#pullrequestreview-1585610771

You could also investigate making your plugin extensible via an ExtensionPoint. So when others see the need for a different implementation, they could write their own plugin that just implements the ExtensionPoint

 

 

From: jenkin...@googlegroups.com <jenkin...@googlegroups.com> On Behalf Of Tomas Bjerre
Sent: Samstag, 13. April 2024 07:13
To: Jenkins Developers <jenkin...@googlegroups.com>
Subject: Jenkins.getInstance().getAllItems(ParameterizedJob.class) slow in large installations

 

You don't often get email from tomas.b...@gmail.com. Learn why this is important

--
You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/cbea2c0c-efa3-4ef2-813b-d8ee3658500an%40googlegroups.com.

[Tomas Bjerre]

Thanks for the input!
I have tried to implement this, a review of the PR would be very appreciated:
https://github.com/jenkinsci/generic-webhook-trigger-plugin/pull/305

You received this message because you are subscribed to a topic in the Google Groups "Jenkins Developers" group.
To unsubscribe from this topic, visit https://groups.google.com/d/topic/jenkinsci-dev/w-jdCAtGh20/unsubscribe.
To unsubscribe from this group and all its topics, send an email to jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/AS1PR02MB78471557F788B420E66A9BB6870B2%40AS1PR02MB7847.eurprd02.prod.outlook.com.

[Jesse Glick]

On Sat, Apr 13, 2024 at 1:13 AM Tomas Bjerre <tomas.b...@gmail.com> wrote:

- Is there a faster way to Jenkins.getInstance().getAllItems(ParameterizedJob.class); ?

Yes, https://javadoc.jenkins.io/hudson/model/ItemGroup.html#allItems(java.lang.Class) is faster since it avoids the need to sort. Also if this code is running as a particular user identity yet the particular block of logic should apply regardless of the user’s permissions to learn of the existence of particular jobs, you can impersonate `ACL.SYSTEM` which will be faster since a typical `AuthorizationStrategy` implementation has a quick path for this principal.

Without knowing what the plugin does or why it is enumerating all projects in a performance-sensitive call site, I cannot offer further advice, but creating a cache smells like a poor solution. Can the plugin be redesigned to look up the required project by name rather than conducting a search?

[Tomas Bjerre]

I implemented and released Markus solution 2024-04-14. There are now 2 issues as a result of this change:

 - https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/311

 - https://github.com/jenkinsci/generic-webhook-trigger-plugin/issues/312

Both issues are scenarios where my ItemListener-implementation:
https://github.com/jenkinsci/generic-webhook-trigger-plugin/blob/3954ee99f8c8801877f6a9b998fe7fdcaa78e6d7/src/main/java/org/jenkinsci/plugins/gwt/jobfinder/JobFinderImpersonater.java#L20

Is not being invoked for the created jobs.

311 seems to be a bug in Job DSL, reported in 2021 here: https://issues.jenkins.io/browse/JENKINS-64553

312 might be a bug in Jenkins core? I could not find an existing Jira but perhaps one should be opened.

To answer Jesse, no I don't think it can be redesigned (without breaking API and forcing many of the 34k users to make big changes). But suggestions on that are very welcome.

[Winter, Markus]

Indeed it sems that when posting an xml to update a job the ItemListener.onUpdated is not called. Only the Saveablelistener is called:

https://github.com/jenkinsci/jenkins/blob/5dfdc5f463b8987d87f5a01754ee45ad58e9ff4c/core/src/main/java/hudson/model/AbstractItem.java#L909

 

While when creating a job then ItemListener.onCreated is called but not the SaveableListener.onChange (well actually it is not a change so this is probably ok)

 

I don’t know how JobDSL works internally but if it is also posting an xml and thus also calling the updateByXML method then it is the same root cause. Implementing also the SaveableListener might do the trick.

I would consider this a bug in core, but maybe others think it works as designed though.

 

From: jenkin...@googlegroups.com <jenkin...@googlegroups.com> On Behalf Of Tomas Bjerre
Sent: Donnerstag, 23. Mai 2024 18:18
To: Jenkins Developers <jenkin...@googlegroups.com>
Subject: Re: Jenkins.getInstance().getAllItems(ParameterizedJob.class) slow in large installations

 

You don't often get email from tomas.b...@gmail.com. Learn why this is important

I implemented and released Markus solution 2024-04-14. There are now 2 issues as a result of this change:

--

You received this message because you are subscribed to the Google Groups "Jenkins Developers" group.
To unsubscribe from this group and stop receiving emails from it, send an email to

jenkinsci-de...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/jenkinsci-dev/5d851a22-9803-4b90-a905-4dca95817531n%40googlegroups.com.

[Jesse Glick]

On Thu, May 23, 2024 at 2:12 PM 'Winter, Markus' via Jenkins Developers <jenkin...@googlegroups.com> wrote:

when posting an xml to update a job the ItemListener.onUpdated is not called. Only the Saveablelistener is called

I have noticed this as well and it feels like a bug. The workaround is to use `SaveableListener` but ignore subjects which are not `Item`s.