Feature/add storm dot graph #236
Conversation
| } | ||
| } | ||
|
|
||
| object StormToplogyBuilder { |
|
some samples of generated graphs |
|
Such a dominator! |
| collectProducers(producer, updatedBolt, updatedDag, forkedNodes, visited = visited) | ||
| } | ||
|
|
||
| def resolve(dependency: Prod[_]): Prod[_] = { |
There was a problem hiding this comment.
add comment here to explain it more.
There was a problem hiding this comment.
Change to boolean: mergeableWithSource()?
Outputs to a graphViz
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
|
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 |
|
|
||
| package com.twitter.summingbird.storm | ||
|
|
||
| import backtype.storm.LocalCluster |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
Removed nearly all imports
| private type Prod[T] = Producer[Storm, T] | ||
| private type VisitedStore = Set[Prod[_]] | ||
|
|
||
| def apply[P](tail: Producer[Storm, P]): StormDag = { |
There was a problem hiding this comment.
this is weird that you have two apply methods that return two different types. They should either be renamed, or return one type.
For apply, I think it works for Function-like objects (one return type), or constructor-like methods (return an instance of the trait or class that the object is a companion of).
There was a problem hiding this comment.
Wait, got cusfused by the inner class with a lowercase name.
|
passed, should be ready to look at changes |
| // Producers which live in the same node will result in a NOP in connect. | ||
| stormNode.members.foldLeft(curDag) { (innerDag, dependantProducer) => | ||
| dependantProducer match { | ||
| case Summer(producer, _, _) => innerDag.connect(producer, dependantProducer) |
There was a problem hiding this comment.
Why not:
Producer
.dependenciesOf(dependantProducer)
.foldLeft(innerDag) { {dag, dep) => dag.connect(dep, dependantProducer) }Would that work and save some code duplication with dependenciesOf?
There was a problem hiding this comment.
Let's address this, if you agree in a next PR.
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