Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-47158

Warnings about workflow/*-parallel-synthetic.xml serializing WorkflowRun objects

    Details

    • Similar Issues:
    • Epic Link:
    • Sprint:
      Blue Ocean - Candidates

      Description

      I get the following message in the log:

      -JENKINS-45892-: reference to Bitbucket/cvltk/PR-4 #9 being saved from unexpected /var/lib/jenkins/jobs/Bitbucket/jobs/cvltk/branches/PR-4/builds/9/workflow/4-parallel-synthetic.xml

      I'm doing some tricks to build in parallel on multiple different nodes, maybe that is the cause. Condensed example Jenkinsfile:

      platforms = [
        "gcc4.8-linux64": [nodeMatcher: 'linux64', config: "linux-gcc4.8"],
      ]
      
      @NonCPS
      def generateBuilds()
      {
        def builds = [:]
        for (p in platforms) {
          def name = p.key
          def platform = p.value
          builds[p.key] = { build(name, platform) }
        }
        return builds
      }
      
      def build(name, platform)
      {
        node(platform.nodeMatcher) {
          stage("Build ${name}") {
            echo "Hello, World for ${platform.config}!"
          }
        }
      }
      
      parallel generateBuilds()
      
      

        Attachments

          Issue Links

            Activity

            Hide
            danielbeck Daniel Beck added a comment -

            Jesse Glick PTAL WDYT?

            Show
            danielbeck Daniel Beck added a comment - Jesse Glick PTAL WDYT?
            Hide
            jglick Jesse Glick added a comment -

            PipelineNodeGraphVisitor and FlowNodeWrapper seem to be doing something crazy I do not understand, and which is potentially inefficient and even dangerous. At a bare minimum, FlowNodeWrapper.run should be transient (and reinjected as needed from whatever code loads it).

            Show
            jglick Jesse Glick added a comment - PipelineNodeGraphVisitor and FlowNodeWrapper seem to be doing something crazy I do not understand, and which is potentially inefficient and even dangerous. At a bare minimum, FlowNodeWrapper.run should be transient (and reinjected as needed from whatever code loads it).
            Hide
            vivek Vivek Pandey added a comment -

            Jesse Glick FlowNodeWrapper is not serialized so not sure what you mean by making it transient.

            Show
            vivek Vivek Pandey added a comment - Jesse Glick FlowNodeWrapper is not serialized so not sure what you mean by making it transient.
            Hide
            jglick Jesse Glick added a comment -

            Vivek Pandey yes it is! You are creating and saving anonymous inner FlowNode subclasses. Trivially reproducible by running 2.77, adding blueocean-web + blueocean-pipeline-rest-impl, creating

            parallel a: {
                node {
                    echo 'a'
                }
            }, b: {
                node {
                    echo 'b'
                }
            }
            

            and running it and opening the result in BO. You will see tons of errors. jobs/…/builds/1/workflow/5-parallel-synthetic.xml contains all sorts of crazy stuff, for example:

            <node class="io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeGraphVisitor$3" plugin="blueocean-pipeline-api-impl@1.2.4">
            

            or

            <run resolves-to="hudson.model.Run$Replacer" plugin="workflow-job@2.14.1">
              <id>…#1</id>
            </run>
            

            (the origin of the warnings)

            or (eeek!)

            <parallelBranches serialization="custom">
              <unserializable-parents/>
              <java.util.ArrayDeque>
            

            etc.

            Show
            jglick Jesse Glick added a comment - Vivek Pandey yes it is! You are creating and saving anonymous inner FlowNode subclasses. Trivially reproducible by running 2.77, adding blueocean-web + blueocean-pipeline-rest-impl , creating parallel a: { node { echo 'a' } }, b: { node { echo 'b' } } and running it and opening the result in BO. You will see tons of errors. jobs/…/builds/1/workflow/5-parallel-synthetic.xml contains all sorts of crazy stuff, for example: <node class= "io.jenkins.blueocean.rest.impl.pipeline.PipelineNodeGraphVisitor$3" plugin= "blueocean-pipeline-api-impl@1.2.4" > or <run resolves-to= "hudson.model.Run$Replacer" plugin= "workflow-job@2.14.1" > <id> …#1 </id> </run> (the origin of the warnings) or (eeek!) <parallelBranches serialization= "custom" > <unserializable-parents/> <java.util.ArrayDeque> etc.
            Hide
            vivek Vivek Pandey added a comment -

            Jesse Glick
            Ah. What its trying to do is to wrap top level parallel block inside a synthetic stage (created using an inner class). Purely for visualization. Code intended to not do a save as its purely a runtime thing. Its getting persisted during addAction, syntheticNode.addAction(new LabelAction(PARALLEL_SYNTHETIC_STAGE_NAME)) is resulting in save. Basically its FlowNode.getActions().add() is causing save.

            Possibly, I can subclass FlowNode and avoid that save, or there is better way I can achieve it?

            Show
            vivek Vivek Pandey added a comment - Jesse Glick Ah. What its trying to do is to wrap top level parallel block inside a synthetic stage (created using an inner class). Purely for visualization. Code intended to not do a save as its purely a runtime thing. Its getting persisted during addAction, syntheticNode.addAction(new LabelAction(PARALLEL_SYNTHETIC_STAGE_NAME)) is resulting in save. Basically its FlowNode.getActions().add() is causing save. Possibly, I can subclass FlowNode and avoid that save, or there is better way I can achieve it?
            Hide
            jglick Jesse Glick added a comment -

            Vivek Pandey I presume you should be operating on some alternate data structure rather than creating virtual FlowNode instances, but I do not really understand the context at all. Sam Van Oort can perhaps advise.

            Show
            jglick Jesse Glick added a comment - Vivek Pandey I presume you should be operating on some alternate data structure rather than creating virtual FlowNode instances, but I do not really understand the context at all. Sam Van Oort can perhaps advise.
            Hide
            svanoort Sam Van Oort added a comment -

            Vivek Pandey I don't understand why you're injecting synthetic nodes?  I wouldn't extend, touch, or reuse the FlowNode class for this because it's pretty tightly coupled to persistence and the data model – also injecting synthetic nodes into your representation of the flow graph is likely to create all sorts of bizarre inconsistencies where your visualization differs from the actual on-disk structure.

            The whole idea behind createParallelSyntheticNode feels rather ill-advised to me for the reasons above – I think what you want to do is decorate your runtime model with some additional information on synthetic stages and add a conditional check against that where appropriate for rendering (without modifying the data model).

            Show
            svanoort Sam Van Oort added a comment - Vivek Pandey I don't understand why you're injecting synthetic nodes?  I wouldn't extend, touch, or reuse the FlowNode class for this because it's pretty tightly coupled to persistence and the data model – also injecting synthetic nodes into your representation of the flow graph is likely to create all sorts of bizarre inconsistencies where your visualization differs from the actual on-disk structure. The whole idea behind createParallelSyntheticNode feels rather ill-advised to me for the reasons above – I think what you want to do is decorate your runtime model with some additional information on synthetic stages and add a conditional check against that where appropriate for rendering (without modifying the data model).
            Hide
            jglick Jesse Glick added a comment -

            Note that I had to add an entry to the stock whitelist in JENKINS-47736 to keep this bug from making BO explode completely.

            Show
            jglick Jesse Glick added a comment - Note that I had to add an entry to the stock whitelist in  JENKINS-47736 to keep this bug from making BO explode completely.
            Hide
            abayer Andrew Bayer added a comment -

            FYI Michael Neale and James Dumay - this is causing major problems on ci.jenkins.io due to the logspam that gets generated. Don't know if it's causing other problems beyond the logspam, but it's filling the disk every couple days, which is bad, obviously. This needs to get fixed ASAP.

            Show
            abayer Andrew Bayer added a comment - FYI Michael Neale and James Dumay - this is causing major problems on ci.jenkins.io due to the logspam that gets generated. Don't know if it's causing other problems beyond the logspam, but it's filling the disk every couple days, which is bad, obviously. This needs to get fixed ASAP.
            Hide
            abayer Andrew Bayer added a comment -

            Ok, I decided to tackle this - https://github.com/jenkinsci/blueocean-plugin/pull/1584. It's a minimal bandaid - just keeps the synthetic FlowNode created in PipelineNodeGraphVisitor for parallel branches from being saved to disk. It doesn't address the underlying design problem of using a synthetic FlowNode in the first place, but it does get Blue Ocean to stop violating JENKINS-45892. Assuming all the tests pass - I honestly haven't run them all locally and am counting on CI. =)

            Jesse Glick - fyi, I couldn't add you as a reviewer to the PR, since you're not a committer on blueocean-plugin, but I wanted to make sure you take a look at it.

            Show
            abayer Andrew Bayer added a comment - Ok, I decided to tackle this - https://github.com/jenkinsci/blueocean-plugin/pull/1584 . It's a minimal bandaid - just keeps the synthetic FlowNode created in PipelineNodeGraphVisitor for parallel branches from being saved to disk. It doesn't address the underlying design problem of using a synthetic FlowNode in the first place, but it does get Blue Ocean to stop violating JENKINS-45892 . Assuming all the tests pass - I honestly haven't run them all locally and am counting on CI. =) Jesse Glick - fyi, I couldn't add you as a reviewer to the PR, since you're not a committer on blueocean-plugin , but I wanted to make sure you take a look at it.
            Hide
            michaelneale Michael Neale added a comment -

            thanks Andrew Bayer - Vivek Pandey will review shortly, but LGTM. 

             

            Should we put this in the release 1.3 branch and roll one if it is bugging people? 

            Show
            michaelneale Michael Neale added a comment - thanks Andrew Bayer - Vivek Pandey will review shortly, but LGTM.    Should we put this in the release 1.3 branch and roll one if it is bugging people? 
            Hide
            michaelneale Michael Neale added a comment -

            this was rolled in 1.3.5 release too FWIW

            Show
            michaelneale Michael Neale added a comment - this was rolled in 1.3.5 release too FWIW
            Hide
            kshultz Karl Shultz added a comment -

            Testing Notes:
            Andrew provided a test as part of the PR.

            Show
            kshultz Karl Shultz added a comment - Testing Notes: Andrew provided a test as part of the PR.

              People

              • Assignee:
                abayer Andrew Bayer
                Reporter:
                estyrke Emil Styrke
              • Votes:
                4 Vote for this issue
                Watchers:
                13 Start watching this issue

                Dates

                • Created:
                  Updated:
                  Resolved: