Generified Flow Search Algorithms #2

svanoort · 2016-04-14T03:47:14Z

Provides a bundle of search and iteration APIs to simplify working with pipeline flow graphs (DAGs).

These are intended to be extensible for custom use cases, but provide a general API that covers what is needed.

For a rundown of the core APIs and how to work with them, see: https://github.com/jenkinsci/workflow-api-plugin/pull/2/files#diff-f8537b8acb35b350992eb264d4f9f207R43

In gist, we're offering some core primitives for working with the flow graph that are needed to do more involved work with the flow graph:

The visitor pattern is the starting point for SAX-like analysis of a flowgraph (and may be extended to provide a block-aware or marker-aware, i.e. stage/milestone-aware visitor)
ForkScanner is needed to effectively handle blocks in the flowgraph (since blocks are seen in a logical linear order). It is the primitive needed for block-based use in a DOM-like tree analysis, or block-aware SAX-like analysis
LinearBlockHoppingFlowScanner is the primitive that allows for quickly ignoring sibling blocks (helpful if you only care about preceding or enclosing nodes, similar to the FlowNodeSerialWalker).

Notes:

Parallel blocks have their own LabelAction for each branch, a ParallelLabelAction on the StepStartNode beginning each branch, with displayname branch + displayname, and getThreadName that returns the label given to the branch, StepEndNode gives the timing too (and final state)

General use cases:

ErrorInfo - [JENKINS-28119] ErrorInfoStep workflow-basic-steps-plugin#2 = https://issues.jenkins-ci.org/browse/JENKINS-28119
- Searching back for node emitting failure (ErrorAction), skips block end node because the blockendnode gets the final error action.
- Then does the walk back via blockjumping to get the nearest log action, and the nearest label too
WorkflowRun.getLogPrefix.
Milestone step - JENKINS-27039
- Needs something like FlowGraphWalker but that bombs out on parallel -> optimal case for recursive breadth-first algorithm (planned but not implemented yet). Caching not allowable probably.
Durable task step: search label/stage which uses a executor node.
- Good use case for caching or incremental caching.
Log search: nearest enclose start node with label for parallel

Pipeline Stage View:

Stage/node labels - the BlockJumpingScanner allows us to solve JENKINS-33290

…ntal analyzer for flow graphs

…the graph

… avoid a cyclic plugin dependency

svanoort · 2016-04-19T10:09:09Z

@jglick When you get a chance, can you give this a peek? It's intended to offer the more general APIs discussed in #1, but also allow for incremental caching of flow graph analysis due to the option to supply StopNodes (so you only analyze up to the last heads).

Probably still a bit rough owing to late-night coding in some cases. But it gives the 3 basic iteration approaches, makes it fairly easy to plug in more (there's one case where we need depth-first-first, and an option to only operate on the enclosing block scopes only would be helpful).

Key points:

we use general-purpose algorithms that can be plugged in with different flow graph walking algorithms
state variables are exposed internally. This is intentional, so we can use and extend this this when plugging this back into the pipeline stage view codebase for more complex stateful analysis.
we can extend the FlowScanners to track more information while walking the graph. For example, storing last node when we iterate, to allow calculating the timings.

Apologies if the above is rambling and/or slightly incoherent. Check the timestamp and you'll see why, I can clarify/explain tomorrow.

jglick · 2016-04-19T16:39:50Z

Intended as a substitute for #1 IIUC.

svanoort · 2016-04-19T17:11:23Z

@jglick Correct, for that and more. I think it probably could use some convenience methods to make it more friendly for that sort of usage, but wanted to solicit input about how that could look.

It may actually be easier for some of these cases (for example, the nested stage/label + excecutor node name combo) to use the visit method, and use its boolean returnVal to control search termination.

jglick · 2016-04-19T19:23:49Z

Another use case is WorkflowRun.getLogPrefix.

svanoort · 2016-04-19T22:20:32Z

Yes, I think so... and, er we don't have to worry about a stack overflow with this mechanism in that case.
Although in that case, I have to wonder if there's a way to simplify the overall log copying.

jglick · 2016-04-21T18:44:55Z

src/test/java/org/jenkinsci/plugins/workflow/graph/TestFlowScanner.java

+
+        scanner = new FlowScanner.DepthFirstScanner();
+        matches = scanner.findAllMatches(heads, null, matchEchoStep);
+        Assert.assertTrue(matches.size() == 5);


assertEquals(5, matches.size());

…can figure out how to get it to use something like semaphore step

…crementally

…e PR

…lockHoppingScanner to indicate it is linear

…r, rewrite internals for iterator, add block test

…ests, fix blockhoppingscanner

…sted parallel blocks and blackListing

jglick · 2016-08-24T14:54:01Z

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java

-        if (parents == null || parents.size() == 0) {
-            // welp done with this node, guess we consult the queue?
+        if (parents.isEmpty()) {
+            // welp, we're done with this node, guess we consult the queue?


for my benefit

jglick · 2016-08-24T14:54:36Z

re:bee:

jglick · 2016-08-24T14:57:23Z

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java.orig

+<<<<<<< HEAD
+    /** If true, we are walking from the flow end node and have a complete view of the flow
+     *  Needed because there are implications when not walking from a finished flow (blocks without a {@link BlockEndNode})*/
+=======


🐛 merge conflicts here and elsewhere

jglick · 2016-08-24T15:06:17Z

pom.xml

@@ -33,7 +33,7 @@
    </parent>
    <groupId>org.jenkins-ci.plugins.workflow</groupId>
    <artifactId>workflow-api</artifactId>
-    <version>2.2-SNAPSHOT</version>
+    <version>2.2-blockapis-SNAPSHOT</version>


Please revert before merging.

@jglick Done.

jglick · 2016-08-24T15:09:20Z

re:bee:

oleg-nenashev · 2016-08-24T15:27:34Z

src/main/java/org/jenkinsci/plugins/workflow/graphanalysis/ForkScanner.java

+    NodeType nextType = null;
+
+    public ForkScanner() {
+


The comment has not been addressed 🐜

@oleg-nenashev I believe it was (check back at the original) -- you can invoke methods on a ForkScanner with no nodes feeding it, they'll return null, empty, or an exception as appropriate.

oleg-nenashev · 2016-08-24T16:59:25Z

So 🐛 for missing myNext reset in /DepthFirstScanner#reset(). I also have serious concerns regarding ForkScanner#setHeads()

Other code looks good to me

…anded just the branch start nodes for a parallel block

svanoort · 2016-08-25T01:41:44Z

@oleg-nenashev That covers the issue with the submitting the Parallel Branch Starts for only an incomplete flow as the heads -- in a fairly straightforward fashion. I guess re-bee potential for @jglick too if he likes

jglick · 2016-08-25T14:13:28Z

re:bee: (do not understand the recent changes but…there is more test code, fine)

oleg-nenashev · 2016-08-25T20:50:14Z

That covers the issue with the submitting the Parallel Branch Starts for only an incomplete flow as the heads -- in a fairly straightforward fashion

Would be useful to have some protective code in setHeads() as well.
But 🐝 , because it seems to address the issue

svanoort · 2016-08-25T20:55:20Z

Final bees, merging for release

svanoort · 2016-08-25T20:55:30Z

@reviewbybees done

Initial implementation of predicate-based flow scanner and an increme…

5f996d5

…ntal analyzer for flow graphs

svanoort added the work-in-progress label Apr 14, 2016

svanoort added 9 commits April 14, 2016 00:34

Pacify FindBugs

ddb608d

Tests for the unrefactored FlowScanner

2b51497

WIP on pulling out generalized flowscanning algorithms

bb0864e

Flesh out/fix most of the flow scanner implementations

f4de174

Finish tests for FlowScanner algorithms and fix remaining edge cases

d414723

Add visitor method to FlowScanner for collecting stats while walking …

4c8faa4

…the graph

Fix FindBugs complaints

eaf4769

Flesh out incremental analysis cache skeleton

0ed42f8

Move FlowScanner method to search by StepDescriptor into test only to…

7f4d488

… avoid a cyclic plugin dependency

jglick changed the title ~~[WIP] Generified Flow Search + Start of Incrementally Updated Flow Stats~~ Generified Flow Search + Start of Incrementally Updated Flow Stats Apr 19, 2016

jglick reviewed Apr 21, 2016
View reviewed changes

svanoort added 5 commits April 25, 2016 17:43

Rename findAllMatches to filter

deb585b

Refactor flow graph incremental analysis, add stub of a test until I …

72e49df

…can figure out how to get it to use something like semaphore step

Fix imports in incremental flow analysis

eb48552

Refactor the incremental flow analysis to allow testing by running in…

e640fd3

…crementally

Remove the incremental flow analysis so we can push it into a separat…

7c1e6df

…e PR

svanoort mentioned this pull request Apr 26, 2016

Incremental flow analysis #3

Closed

4 tasks

svanoort added 2 commits April 26, 2016 23:56

Start a ForkFlowScanner (WIP), add lots of Javadoc/comments, rename B…

6dfbb2d

…lockHoppingScanner to indicate it is linear

Syntactic sugar and comments on FlowScanner

ae9581b

svanoort mentioned this pull request Apr 27, 2016

Flowscanner reimplement jenkins28119 error info jenkinsci/workflow-basic-steps-plugin#3

Draft

svanoort added 3 commits April 30, 2016 14:14

FlowScanner: switch to simpler iterator use internally, add filterato…

1331ddd

…r, rewrite internals for iterator, add block test

More AbstractFlowScanner helpers, add iterator on block starts, add t…

a307266

…ests, fix blockhoppingscanner

FlowScanning: ForkScanner passes in basic case, next up is testing ne…

211f297

…sted parallel blocks and blackListing

jglick reviewed Aug 24, 2016
View reviewed changes

Javadoc error.

4cc9206

jglick reviewed Aug 24, 2016
View reviewed changes

svanoort added 2 commits August 24, 2016 10:57

Solved a javadoc hiccup

2bcfd8b

Remove accidentally commit merge files

34483c3

jglick reviewed Aug 24, 2016
View reviewed changes

Revert special versioning in prep for release

78e8d2c

oleg-nenashev reviewed Aug 24, 2016
View reviewed changes

svanoort added 2 commits August 24, 2016 13:04

Fix a missing DepthFirstScanner reset of field

8abdf34

Fix and thoroughly test against a bug with ForkScanner failing when h…

3d3bf57

…anded just the branch start nodes for a parallel block

svanoort merged commit 1d7b255 into master Aug 25, 2016

jglick deleted the generic-flow-analyzer branch February 1, 2017 22:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Generified Flow Search Algorithms #2

Generified Flow Search Algorithms #2

svanoort commented Apr 14, 2016 •

edited

Loading

svanoort commented Apr 19, 2016

jglick commented Apr 19, 2016

svanoort commented Apr 19, 2016 •

edited

Loading

jglick commented Apr 19, 2016 •

edited by svanoort

Loading

svanoort commented Apr 19, 2016

jglick Apr 21, 2016

jglick Aug 24, 2016

jglick commented Aug 24, 2016

jglick Aug 24, 2016

jglick Aug 24, 2016

svanoort Aug 24, 2016

jglick commented Aug 24, 2016

oleg-nenashev Aug 24, 2016

svanoort Aug 24, 2016

oleg-nenashev commented Aug 24, 2016

svanoort commented Aug 25, 2016 •

edited

Loading

jglick commented Aug 25, 2016

oleg-nenashev commented Aug 25, 2016

svanoort commented Aug 25, 2016

svanoort commented Aug 25, 2016

Generified Flow Search Algorithms #2

Generified Flow Search Algorithms #2

Conversation

svanoort commented Apr 14, 2016 • edited Loading

svanoort commented Apr 19, 2016

jglick commented Apr 19, 2016

svanoort commented Apr 19, 2016 • edited Loading

jglick commented Apr 19, 2016 • edited by svanoort Loading

svanoort commented Apr 19, 2016

jglick Apr 21, 2016

Choose a reason for hiding this comment

jglick Aug 24, 2016

Choose a reason for hiding this comment

jglick commented Aug 24, 2016

jglick Aug 24, 2016

Choose a reason for hiding this comment

jglick Aug 24, 2016

Choose a reason for hiding this comment

svanoort Aug 24, 2016

Choose a reason for hiding this comment

jglick commented Aug 24, 2016

oleg-nenashev Aug 24, 2016

Choose a reason for hiding this comment

svanoort Aug 24, 2016

Choose a reason for hiding this comment

oleg-nenashev commented Aug 24, 2016

svanoort commented Aug 25, 2016 • edited Loading

jglick commented Aug 25, 2016

oleg-nenashev commented Aug 25, 2016

svanoort commented Aug 25, 2016

svanoort commented Aug 25, 2016

svanoort commented Apr 14, 2016 •

edited

Loading

svanoort commented Apr 19, 2016 •

edited

Loading

jglick commented Apr 19, 2016 •

edited by svanoort

Loading

svanoort commented Aug 25, 2016 •

edited

Loading