Skip to content
This repository was archived by the owner on Jan 20, 2022. It is now read-only.

Feature/add storm dot graph#236

Merged
johnynek merged 17 commits intodevelopfrom
feature/add_storm_dot_graph
Sep 22, 2013
Merged

Feature/add storm dot graph#236
johnynek merged 17 commits intodevelopfrom
feature/add_storm_dot_graph

Conversation

@ianoc
Copy link
Copy Markdown
Contributor

@ianoc ianoc commented Sep 20, 2013

Adds the concept of a Planned DAG for storm jobs that can be emitted as a graphviz

Adds a series of laws for this DAG

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

typo

@ianoc
Copy link
Copy Markdown
Contributor Author

ianoc commented Sep 20, 2013

failinggraph42 dot
failinggraph36 dot

some samples of generated graphs

@sritchie
Copy link
Copy Markdown

Such a dominator!

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

add comment here to explain it more.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Change to boolean: mergeableWithSource()?

Add law saying no flat maps can live with a source
Adds extra handling in graph to ensure we don't push flat mapped producers down to the source
Rename forward/reverse to dependsOn and dependantOf for clarity
Add comments for maybeCollapse and maybeSplit
…hose producers aren't normal merges.

Add in a block to disallow a source to be both sides of a merge(or indeed any other node).

Fix generator so this doesn't occur.

Clean up and encapsulate in a private class more of the separation logic
@ianoc
Copy link
Copy Markdown
Contributor Author

ianoc commented Sep 21, 2013

Updated to fix an edge case where the MergeProducer that was being depended on by another MergeProducer was a fan out node and so couldn't be collapsed down. Also where the visited nodes having been seen was causing some StormNodes not to be registered

@ianoc
Copy link
Copy Markdown
Contributor Author

ianoc commented Sep 21, 2013

@johnynek , @sritchie The mega graphs its making now:

Though note the nodes which are just IdentityKeyedProducer and MergedProducers
failinggraph5433 dot

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are any of these used? (storm imports)?

Can you audit and remove anything unnneed? It will make it easier to think of using this on other platforms.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removed nearly all imports

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants