This repository has been archived by the owner on Dec 15, 2021. It is now read-only.
[JENKINS-30055] Handle a large number of steps in quick succession #213
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…must receive events immediately, and unregister itself.
…g build.xml. java.lang.Thread.State: RUNNABLE at java.io.UnixFileSystem.delete0(Native Method) at java.io.UnixFileSystem.delete(UnixFileSystem.java:265) at java.io.File.delete(File.java:1041) at hudson.Util.deleteFile(Util.java:221) at hudson.util.AtomicFileWriter.commit(AtomicFileWriter.java:113) at hudson.XmlFile.write(XmlFile.java:179) at hudson.model.Run.save(Run.java:1898) - locked <0x0000000707031450> (a org.jenkinsci.plugins.workflow.job.WorkflowRun) at org.jenkinsci.plugins.workflow.job.WorkflowRun$GraphL.onNewHead(WorkflowRun.java:664) at org.jenkinsci.plugins.workflow.cps.CpsFlowExecution.notifyListeners(CpsFlowExecution.java:790)
|
This pull request originates from a CloudBees employee. At CloudBees, we require that all pull requests be reviewed by other CloudBees employees before we seek to have the change accepted. If you want to learn more about our process please see this explanation. |
… resaving build.xml." Did not have the intended effect, because there is typically just one listener, and the problem is notifyListeners being called many times. This reverts commit a399362.
FlowNode.equals is based on id, so a static cache will be incorrect.
…wRun.save can be called from various threads. The mutations are all guarded by WorkflowRun.completed, so we just need to allow serialization to see a snapshot. (There was already a race condition in the case of abrupt shutdown, but at worst these should result in duplicated log text.)
| p.setDefinition(new CpsFlowDefinition("for (int i = 0; i < 10000; i++) {echo \"iteration #${i}\"}", true)); | ||
| WorkflowRun b = r.assertBuildStatusSuccess(p.scheduleBuild2(0)); | ||
| r.assertLogContains("iteration #9876", b); | ||
| } |
There was a problem hiding this comment.
The problem, as I remember it, only became visible after a restart. In that case, shouldn't the test try reloading the persisted form of the Run (WorkflowRun.onLoad etc)?
Indeed I just noticed this aspect of your JENKINS-30055. Perhaps not related to JENKINS-30651 after all. Looking into it. |
…aps independent of JENKINS-30651.
|
🐝 on wip as my comment is addressed. |
|
|
… makes it faster. Have not yet figured out how to load old flow node files.
|
Currently |
(BulkChange calls save even if no attempt to save was made. And from a synchronous listener you should not be saving.)
| @@ -391,24 +394,48 @@ private void copyLogs() { | |||
| } | |||
| } | |||
|
|
|||
| // CacheBuilder would be more convenient, but apparently does not support null values. Memoizer does not support weak keys. | |||
There was a problem hiding this comment.
Could use Optional but seems like overkill.
There was a problem hiding this comment.
What blocks from using a FillerObject instead of null?
There was a problem hiding this comment.
Hence my comment about Optional.
| public abstract void addListener(GraphListener listener); | ||
|
|
||
| public /*abstract*/ void removeListener(GraphListener listener) {} |
There was a problem hiding this comment.
Needs Javadoc and "TODO: make abstract" for the next backward-incompatible version
There was a problem hiding this comment.
There is no planned incompatible version.
There was a problem hiding this comment.
backward-what? There is no such word in Jenkins vocabulary 😺
|
🐝 |
|
LGTM 🐝 |
jglick
added a commit
that referenced
this pull request
Jan 11, 2016
[JENKINS-30055] Handle a large number of steps in quick succession
This was referenced Jan 12, 2016
Closed
2 tasks
jglick
added a commit
to jglick/jenkins
that referenced
this pull request
Jan 4, 2018
…ine-plugin#213 as demonstrated by CoberturaPublisherPipelineTest with jenkinsci/cobertura-plugin#90.
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
JENKINS-30055
Performance is still not great (about a minute to run on my laptop); mostly it just seems to be waiting in disk I/O to write lots of little log files, which is unavoidable so long as we maintain separate logs per step (JENKINS-30896).
@reviewbybees