-
Improvement
-
Resolution: Fixed
-
Major
Currently we always load FlowExecution when loading a build even though we might not actually need it. This is thought to slow down some things, such as getBuildHealth, since we are loading executions for some historical completed builds and then not using them. The loading should be on demand only.
Since currently FlowExecution.onComplete is called soon thereafter, we would need some other marker in WorkflowRun for a completed build. Could check for logsToCopy == null though this is deleted in JENKINS-38381. Probably better to use completed as noted here.
- is duplicated by
-
JENKINS-47224 Pipeline builds failing/hanging under load: RejectedAccessException: Scripts not permitted to use staticMethod WorkflowScript ___cps___6009
-
- Closed
-
-
-
- Closed
-
- is related to
-
-
- Closed
-
- relates to
-
-
- Resolved
-
-
-
- Closed
-
- links to
[JENKINS-45585] WorkflowRun.onLoad need not eagerly load the FlowExecution of a completed build
| Remote Link | New: This issue links to "CloudBees Internal CD-20 (Web Link)" [ 19118 ] |
| Link |
New:
This issue relates to |
| Link |
New:
This issue relates to |
jglick - it looks we set state = State.COMPLETED via onEndBuilding() in finish(Result,Throwable) - could we just use that as the basis for deciding what to set completed to early in onLoad?
jglick - so I may be crazy, but it feels like if we were to add CpsFlowExecution#done to CpsFlowExecution.ConverterImpl's marshal and unmarshal methods (it's unpersisted currently), we could do a CpsFlowExecution#isComplete() call in WorkflowRun#onLoad() before we end up calling CpsFlowExecution#onLoad... WorkflowRun#execution would exist correctly, though it wouldn't yet have the heads set...hrm. I dunno, actually. I may be talking myself out of this.
Thinking yet more: I think CpsFlowExecution#done should be in CpsFlowExecution.ConverterImpl#marshal and #unmarshal - it's weird that it's not persisted currently, at least to me. With that in place, CpsFlowExecution#isComplete() would be true for completed builds at load time. CpsFlowExecution#onLoad(FlowExecutionOwner) would call initializeStorage() and load the flow nodes, but not go into loadProgramAsync and onwards to parsing/transformation/etc. That at least deals with some of the issues caused by WorkflowRun#execution being eagerly loaded. So then the question is whether we want to bother with the initializeStorage() call at all in lazy-loading situations. Assuming that we don't want to do that, then we could change to something like this in WorkflowRun#onLoad:
FlowExecution fetchedExecution = execution;
if (fetchedExecution != null && !fetchedExecution.isComplete()) {
try {
if (getParent().isResumeBlocked() && execution instanceof BlockableResume) {
((BlockableResume) execution).setResumeBlocked(true);
}
fetchedExecution.onLoad(new Owner(this));
} catch (Exception x) {
LOGGER.log(Level.WARNING, null, x);
execution = null; // probably too broken to use
}
}
fetchedExecution = execution;
if (fetchedExecution != null) {
...
...i.e., skipping calling fetchedExecution.onLoad(new Owner(this)) for a completed FlowExecution. The following if block already has a check for !fetchedExecution.isComplete() before doing some of its body, so we probably wouldn't need to do anything there, I guess?
Sam Van Oort
made changes -
| Assignee | New: Sam Van Oort [ svanoort ] |
Code changed in jenkins
User: Jesse Glick
Path:
src/main/java/org/jenkinsci/plugins/workflow/job/WorkflowRun.java
http://jenkins-ci.org/commit/workflow-job-plugin/bbec76226dcbd01622a6f29a753d1e72af49e03f
Log:
Noting
JENKINS-45585.