Skip to content

Optimize JnlpAgentReceiver to use O(1) cache lookup - #27231

Closed
aryawadhwa wants to merge 4 commits into
jenkinsci:masterfrom
aryawadhwa:fix-jnlp-agent-lookup
Closed

Optimize JnlpAgentReceiver to use O(1) cache lookup#27231
aryawadhwa wants to merge 4 commits into
jenkinsci:masterfrom
aryawadhwa:fix-jnlp-agent-lookup

Conversation

@aryawadhwa

Copy link
Copy Markdown

This PR optimizes the node lookup in JnlpAgentReceiver for inbound agents. It changes an $O(N)$ linear search over all nodes to an $O(1)$ lookup using Jenkins.get().getComputer(clientName). This improves performance when a large number of nodes are connected.

This is a follow-up optimization to the inbound agent secret rotation changes.

Testing done

  • Tested with mvn test in the jenkins/slaves module to ensure no regressions in JNLP receiver logic.
  • Interactive Testing Note: As requested by reviewers, this should be tested interactively on a local Jenkins instance with multiple inbound agents to ensure agent connections are properly verified and secrets are matched correctly. (I am running this locally to verify before marking ready).

Proposed changelog entries

  • Optimize JNLP agent connection lookup to use O(1) computer cache instead of O(N) node iteration

Proposed changelog category

/label rfe

Proposed upgrade guidelines

N/A

Submitter checklist

  • The issue, if it exists, is well-described.
  • The changelog entries and upgrade guidelines are appropriate for the audience affected by the change (users or developers, depending on the change) and are in the imperative mood.
  • There is automated testing or an explanation as to why this change has no tests.
  • New public classes, fields, and methods are annotated with @Restricted or have @since TODO Javadocs, as appropriate.
  • New deprecations are annotated with @Deprecated(since = "TODO") or @Deprecated(forRemoval = true, since = "TODO"), if applicable.
  • UI changes do not introduce regressions when enforcing the current default rules of Content Security Policy Plugin.
  • For dependency updates, there are links to external changelogs and, if possible, full differentials.
  • For new APIs and extension points, there is a link to at least one consumer.

Desired reviewers

@MarkEWaite

… rotation

- Use static SECURE_RANDOM field in Slave to avoid DMI_RANDOM_USED_ONLY_ONCE
- Replace vacuous instanceof Slave checks with null-checks in
  SlaveComputer.getJnlpMac() and doRevokeInboundSecret() (BC_VACUOUS_INSTANCEOF)
- Add 3 additional tests: unique salt per agent, HTTP revoke endpoint,
  backward-compat fallback for agents without a salt
Signed-off-by: aryawadhwa <aryawadhwa2@gmail.com>
@comment-ops-bot comment-ops-bot Bot added the rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted label Aug 12, 2026
@MarkEWaite

Copy link
Copy Markdown
Contributor

I'm not persuaded that this is valuable enough to justify the added complexity. Even with hundreds of nodes, the lookup time will be indistinguishable between a linear lookup and a hashed lookup.

The pull request includes unrelated changes that must be removed. It is probably best to raise an issue in the issue tracker and describe the problem that you're trying to solve. Provide examples of the measurements that you've used to confirm there is a problem.

Closing because it includes unrelated changes and needs more discussion before it is implemented.

@MarkEWaite MarkEWaite closed this Aug 12, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

rfe For changelog: Minor enhancement. use `major-rfe` for changes to be highlighted

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants