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

Single running parallel branch missing parent parallel block.

    • Icon: Bug Bug
    • Resolution: Unresolved
    • Icon: Minor Minor
    • None

      When there is a single, running branch of a pipeline, the StandardChunkVisitor doesn't trigger `parallelEnd` or `parallelStart` for it's parent block.

      This works:

      node {
          parallel([
              "A": {
                  stage("Build") {
                      echo("Build A")
                  }
                  stage("Test") {
                      parallel([
                          "A1" : {
                             echo("Test A1")
                          },
                          "A2" : {
                             echo("Test A2")
                          }
                      ])
                  }
              },
              "B": {
                  stage("Build") {
                      echo("Build B")
                  }
                  parallel([
                      "B1" : {
                         echo("Test B1")
                         sleep(60)
                      },
                      "B2" : {
                         echo("Test B2")
                         sleep(60;
                      }
                  ])
              }
          ])
      } 
      

      We get `A<-A1` and `B<-B1,B2`.

      This doesn't:

      node {
          parallel([
              "A": {
                  stage("Build") {
                      echo("Build A")
                  }
                  stage("Test") {
                      parallel([
                          "A1" : {
                             echo("Test A1")
                          },
                          "A2" : {
                             echo("Test A2")
                          }
                      ])
                  }
              },
              "B": {
                  stage("Build") {
                      echo("Build B")
                  }
                  parallel([
                      "B1" : {
                         echo("Test B1")
                         sleep(60)
                      },
                  ])
              }
          ])
      } 
      

      (`B1` becomes a sibling of `A` - `B` is not listed in the graph until the build completes).

      It feels like this would have been reported already, but I can't find an issue - so raising just in case.

      Update:
      More specifically, in the second Pipeline - `parallelBranchEnd/parallelBranchStart` don't get called for `B` whilst the Pipeline is still executing. This is not the case once the pipeline is finished or in the first example.

          [JENKINS-72771] Single running parallel branch missing parent parallel block.

          Tim Jacomb added a comment -

          (jglick / Devin (can't find his jira name) appear to be the maintainer)

          This came from https://github.com/jenkinsci/pipeline-graph-view-plugin/pull/285

          Tim Jacomb added a comment - ( jglick / Devin (can't find his jira name) appear to be the maintainer) This came from https://github.com/jenkinsci/pipeline-graph-view-plugin/pull/285

          Jesse Glick added a comment -

          dnusbaum that is

          Jesse Glick added a comment - dnusbaum that is

          Devin Nusbaum added a comment - - edited

          IMO, ForkScanner and related APIs like ChunkFinder are just fundamentally broken for non-trivial cases, particularly when used on incomplete builds, and should be avoided. I would also probably avoid pipeline-graph-analysis in general in newly written code.

          Instead, I would use DepthFirstScanner as mentioned in https://github.com/jenkinsci/workflow-api-plugin/pull/306.

          To elaborate, I found a few serious issues last year while trying to make ForkScanner work with a proprietary Pipeline visualization I worked on (see the video on https://docs.cloudbees.com/docs/cloudbees-ci/latest/pipelines/cloudbees-pipeline-explorer-plugin, timestamp roughly 2:08 shows the relevant functionality), including https://github.com/jenkinsci/workflow-api-plugin/pull/287, https://github.com/jenkinsci/workflow-api-plugin/pull/302 (that might be the case in the description here), and https://github.com/jenkinsci/workflow-api-plugin/pull/305. Maybe with enough time, someone could find a way to make things work in all of the edge cases, but for me it was just way simpler to use DepthFirstScanner directly to avoid all of the problems.

          The core logic in FlowGraphTable might be useful as a starting point just to show you how to build up a basic tree, but would need to be adapted for your specific use case (e.g. filtering out stuff besides stage, parallel, and parallel branches).

          Devin Nusbaum added a comment - - edited IMO, ForkScanner and related APIs like ChunkFinder are just fundamentally broken for non-trivial cases, particularly when used on incomplete builds, and should be avoided. I would also probably avoid pipeline-graph-analysis in general in newly written code. Instead, I would use DepthFirstScanner as mentioned in https://github.com/jenkinsci/workflow-api-plugin/pull/306 . To elaborate, I found a few serious issues last year while trying to make ForkScanner work with a proprietary Pipeline visualization I worked on (see the video on https://docs.cloudbees.com/docs/cloudbees-ci/latest/pipelines/cloudbees-pipeline-explorer-plugin , timestamp roughly 2:08 shows the relevant functionality), including https://github.com/jenkinsci/workflow-api-plugin/pull/287 , https://github.com/jenkinsci/workflow-api-plugin/pull/302 (that might be the case in the description here), and https://github.com/jenkinsci/workflow-api-plugin/pull/305 . Maybe with enough time, someone could find a way to make things work in all of the edge cases, but for me it was just way simpler to use DepthFirstScanner directly to avoid all of the problems. The core logic in FlowGraphTable might be useful as a starting point just to show you how to build up a basic tree, but would need to be adapted for your specific use case (e.g. filtering out stuff besides stage , parallel , and parallel branches).

          Tim Brown added a comment -

          > IMO, ForkScanner and related APIs like ChunkFinder are just fundamentally broken for non-trivial cases

          If this is the case, wouldn't it be best to mark it deprecated so people don't try and use it?

          Tim Brown added a comment - > IMO, ForkScanner and related APIs like ChunkFinder are just fundamentally broken for non-trivial cases If this is the case, wouldn't it be best to mark it deprecated so people don't try and use it?

          Tim Brown added a comment -

          Are there docs or a good example of something creating a graph using `DepthFirstScanner`?

          FWIW PipelineGraphView uses SimpleChunkVisitor because BlueOcean did - I think everyone would prefer a simpler/more reliable method.

          Tim Brown added a comment - Are there docs or a good example of something creating a graph using `DepthFirstScanner`? FWIW PipelineGraphView uses SimpleChunkVisitor because BlueOcean did - I think everyone would prefer a simpler/more reliable method.

          Devin Nusbaum added a comment - - edited

          If this is the case, wouldn't it be best to mark it deprecated so people don't try and use it?

          Yes, probably so. I considered doing that in https://github.com/jenkinsci/workflow-api-plugin/pull/306 when I added the warning to the Javadoc, but it does work ok for completed builds as far as I know, so I wasn't sure about it.

          Are there docs or a good example of something creating a graph using `DepthFirstScanner`?

          The best available example for this case is I think FlowGraphTable like I mentioned (I think that code can be simplified depending on your requirements, but you might be able to use it directly). Basically the idea is to traverse all of the nodes to create a tree, filtering it as needed, and then you then visualize it as desired (e.g. as a tree directly, or flip it on its side to get something like Blue Ocean). (Also, depending on how you want your visualization to work, maybe building a tree is unnecessary. For example, if you are want to display circles for stages/parallel branches with lines between them, maybe you can just iterate over the graph directly, adding metadata as needed, and then use something like d3 to build the layout automatically.)

          FWIW PipelineGraphView uses SimpleChunkVisitor because BlueOcean did

          Yes, I was not around for the original creation of Blue Ocean, so I do not know why it was implemented the way it was, but I would not use it as a reference for new code.

          Devin Nusbaum added a comment - - edited If this is the case, wouldn't it be best to mark it deprecated so people don't try and use it? Yes, probably so. I considered doing that in https://github.com/jenkinsci/workflow-api-plugin/pull/306 when I added the warning to the Javadoc, but it does work ok for completed builds as far as I know, so I wasn't sure about it. Are there docs or a good example of something creating a graph using `DepthFirstScanner`? The best available example for this case is I think FlowGraphTable like I mentioned (I think that code can be simplified depending on your requirements, but you might be able to use it directly). Basically the idea is to traverse all of the nodes to create a tree, filtering it as needed, and then you then visualize it as desired (e.g. as a tree directly, or flip it on its side to get something like Blue Ocean). (Also, depending on how you want your visualization to work, maybe building a tree is unnecessary. For example, if you are want to display circles for stages/parallel branches with lines between them, maybe you can just iterate over the graph directly, adding metadata as needed, and then use something like d3 to build the layout automatically.) FWIW PipelineGraphView uses SimpleChunkVisitor because BlueOcean did Yes, I was not around for the original creation of Blue Ocean, so I do not know why it was implemented the way it was, but I would not use it as a reference for new code.

          Tim Brown added a comment - - edited

          HI Devin,

          Thanks for your suggestion of using the DepthFirstScanner (it's something I looked at before, but assume was wasn't used as it an older/slower traversal method). I found it better (for me) to get the nodes, order them by id and iterate over them - the DepthFirstScanner sent a lot of nodes to the end of the iteration, which tripped me up for a while.

          I've updated my PR to move the Graph View to use new graph building method built on DepthFirstScanner: https://github.com/jenkinsci/pipeline-graph-view-plugin/pull/285 (feel free to comment).

          Also (for those finding this bug in the future) the readme in worflow-api-plugin is useful - and it's 100% right about needing to read the StatusAndTiming API in order to use it properly.

          Tim Brown added a comment - - edited HI Devin, Thanks for your suggestion of using the DepthFirstScanner (it's something I looked at before, but assume was wasn't used as it an older/slower traversal method). I found it better (for me) to get the nodes, order them by id and iterate over them - the DepthFirstScanner sent a lot of nodes to the end of the iteration, which tripped me up for a while. I've updated my PR to move the Graph View to use new graph building method built on DepthFirstScanner: https://github.com/jenkinsci/pipeline-graph-view-plugin/pull/285 (feel free to comment). Also (for those finding this bug in the future) the readme in worflow-api-plugin is useful - and it's 100% right about needing to read the StatusAndTiming API in order to use it properly.

          Devin Nusbaum added a comment -

          Oh, yeah sorry I forgot about that readme. FWIW, I would be wary of using `StatusAndTiming.computeChunkStatus2`. As I recall, it more or less assumes that you are passing nodes exactly how `ForkScanner.visitSimpleChunks` would give them to you, so if you pass other things, the behavior may be anomalous. If it works for you though, great. If you run into issues, I would extract whatever parts of its logic you care about for your visualization into your plugin so that you can control the behavior.

          Devin Nusbaum added a comment - Oh, yeah sorry I forgot about that readme. FWIW, I would be wary of using `StatusAndTiming.computeChunkStatus2`. As I recall, it more or less assumes that you are passing nodes exactly how `ForkScanner.visitSimpleChunks` would give them to you, so if you pass other things, the behavior may be anomalous. If it works for you though, great. If you run into issues, I would extract whatever parts of its logic you care about for your visualization into your plugin so that you can control the behavior.

            Unassigned Unassigned
            canuck1987 Tim Brown
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

              Created:
              Updated: