Optimizations & simplifications pertaining to lazy-loading

[jglick]

Actually three independent changes which could be filed separately if desired:

• Avoid TransientActionFactory from Job.getPermalinks
• Optimizing PeepholePermalink.RunListener to avoid resolving old targets
• Simplified RunMixIn handling of next/previous build

While investigating the behavior of lazy build loading in CloudBees CI in HA mode I found several reasons why running a new build would wind up forcing the last (typically completed) build to be loaded into memory. I had worked on this a couple of years ago and filed #7998 but noted at that time that there were various outstanding issues. One I actually fixed recently in many cases in #11293 but forgot to mention the linkage before.

One class of problems stemmed from permalinks. jenkinsci/scm-api-plugin#358 shows a typical stack trace. First, Job.getPermalinks as of #1059 was going through TransientActionFactory. This actually seems to have been gratuitous. PermalinkProjectAction (rarely implemented) says it is

attached to AbstractProject (through JobProperty.getJobActions)

which is in fact called only by AbstractProject.createTransientActions, and so far as I can tell never by WorkflowJob. So the transient action list for a non-Pipeline project would already have had any attached PermalinkProjectAction, and a Pipeline project would not have supported these to begin with.

Then there was a listener checking for a build completing or being deleted and updating any relevant permalinks. This was forcing resolution of the build previously referred to by the permalink, and perhaps updating the permalink. In fact this logic only needs to know the build number previously targeted. Especially after the refactoring in #10042, it is straightforward to just retrieve that number, and not bother loading the Run itself if it was not already in memory.

Finally, #1191 specifically forced the previous build to be loaded when starting a new one. This was particularly unwanted since it happens while holding Queue.lock. The code here was tricky to follow and the history is a bit torturous: c32f006 in #573 introducing lazy loading; f024f59 and b39a779 in #1146 supporting Pipeline; #1190; b6226ad (no PR) amending that. The lazy loading system was maintaining a doubly-linked list of build references, populated on demand by the standard forward/backward search functions, with the concomitant complexity of maintaining this cache in the face of build deletion. (There appears to be no synchronization here so I suspect the list could be corrupted under certain timing conditions.) A comment implied that all this was necessary to support removing a build from RunMap, but I see no evidence that was ever the case. I have simplified all this to drop the data structure and just look up the next/previous build directly if and when requested, using the AbstractLazyLoadRunMap systems already in place to support such traversals. Note that ExternalRun from https://plugins.jenkins.io/external-monitor-job/ appears to have lacked next/previous links for at least a decade and apparently nobody complained.

Testing done

Automated tests pass with these changes in CloudBees CI.

A CI-specific test that counts the number of builds loaded into memory by a given HA replica was able to be strengthened to confirm that only the builds actually being run by that replica were loaded. I expect the same could apply in any Jenkins controller though the setup would require RunLoadCounter.

Proposed changelog entries

• N/A

Proposed changelog category

/label internal

Proposed upgrade guidelines

N/A

Maintainer checklist

• There are at least two (2) approvals for the pull request and no outstanding requests for change.
• Conversations in the pull request are over, or it is explicit that a reviewer is not blocking the change.
• Changelog entries in the pull request title and/or Proposed changelog entries are accurate, human-readable, and in the imperative mood.
• Proper changelog labels are set so that the changelog can be generated automatically.
• If the change needs additional upgrade steps from users, the upgrade-guide-needed label is set and there is a Proposed upgrade guidelines section in the pull request title (see example).
• If it would make sense to backport the change to LTS, a Jira issue must exist, be a Bug or Improvement, and be labeled as lts-candidate to be considered (see query).

[MarkEWaite]

This PR is now ready for merge. We will merge it after approximately 24 hours if there is no negative feedback.

/label ready-for-merge