From 5528449a64ca7084c070b3082ccebf41536e789a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Toma=C5=BE=20Jerman?= Date: Sat, 22 Aug 2020 18:39:52 +0200 Subject: [PATCH] Enhance graph node's execution context Each node receives their parent and child nodes for a simpler exec context logic. --- pkg/envoy/graph.go | 132 +++++++++++++++++++++++++++++++--------- pkg/envoy/graph_test.go | 94 +++++++++++++--------------- 2 files changed, 143 insertions(+), 83 deletions(-) diff --git a/pkg/envoy/graph.go b/pkg/envoy/graph.go index fffc3adf6..7de6268f2 100644 --- a/pkg/envoy/graph.go +++ b/pkg/envoy/graph.go @@ -17,8 +17,8 @@ type ( // When it comes to larger setups (custom CRM) the number of nodes and relations is well below 1000. // Due to the simple interface, we can define a more optimal graph implementation as a sepperate module. Graph struct { - // Nodes defines a set of all available structures conforming to the Node interface - Nodes []Node + // nodes defines a set of all available structures conforming to the Node interface + nodes []Node // Since everything is calculated on-the-fly, we need a simple boolean flag to determine if the graph is inverted invert bool // Allows us to keep track of all the nodes that were determined as conflicting. @@ -26,11 +26,10 @@ type ( // If the data input is consistent, the conflicting nodes will be consistent, but there is no guarantee // what node in a cycle will be selected as a conflicting. conflicts set + // Allows us to easily keep track of nodes that were already processed. + processed set } - // NodeRelation specifies what nodes from what resource this node is in relation with. - // The mapping is in the form of { resource: [nodeID, nodeID, ...] } - NodeRelation map[string][]string // A simple "set" implementation for simpler, quicker checks set map[string]bool @@ -48,42 +47,74 @@ type ( // Relations provides a list of node's relations // This can be calculated on the fly based on the node's state and don't need to be // built in to the node struct. - Relations() NodeRelation + Relations() map[string][]string // Run should implement the actual operation that should be performend when the node is invoked. // This can be as simple or as complex as needed - Run(context.Context) error + // It's also provided a set of child and parent nodes so we can easily provide node context + Run(ctx context.Context, cc []Node, pp []Node) error // ResolveConflict should implement any operation that should occur when the node // causes a dependency conflict. For example -- partially import the records (without relations) // and correct those relations when executing the Run function - ResolveConflict(context.Context) error + // It's also provided a set of child and parent nodes so we can easily provide node context + ResolveConflict(ctx context.Context, cc []Node, pp []Node) error } ) // Add registers a given set of nodes nn into the graph g func (g *Graph) Add(nn ...Node) { - g.Nodes = append(g.Nodes, nn...) + g.nodes = append(g.nodes, nn...) } // Remove removes the set of nodes nn from the graph h func (g *Graph) Remove(nn ...Node) { - mm := make([]Node, 0, len(g.Nodes)-len(nn)) - for _, m := range g.Nodes { - mh := NodeHash(m) + if len(nn) <= 0 { + return + } + + mm := make([]Node, 0, len(g.nodes)) + for _, m := range g.nodes { for _, n := range nn { - if mh != NodeHash(n) { - mm = append(mm, m) + if NodeHash(m) == NodeHash(n) && g.canRemove(n) { + goto skip } } + mm = append(mm, m) + + skip: + } + g.nodes = mm +} + +func (g *Graph) canRemove(n Node) bool { + if len(g.nodes) <= 1 { + return true + } + + // When the first unprocessed child node is found, terminate + for _, m := range g.Children(n) { + if !g.processed[NodeHash(m)] { + return false + } + } + + return true +} + +func (g *Graph) MarkProcessed(nn ...Node) { + if g.processed == nil { + g.processed = make(set) + } + for _, n := range nn { + g.processed[NodeHash(n)] = true } - g.Nodes = mm } // FindNode returns all nodes that match the given resource and identifiers func (g *Graph) FindNode(res string, IDs ...string) []Node { nn := make([]Node, 0, len(IDs)) - for _, n := range g.Nodes { + for _, n := range g.nodes { for _, ID := range IDs { if NodeHash(n) == NodeHashRaw(res, ID) { nn = append(nn, n) @@ -108,15 +139,24 @@ func (g *Graph) SetNodeConflict(n Node) { g.conflicts[NodeHash(n)] = true } +func (g *Graph) removeProcessed(nn []Node) []Node { + mm := make([]Node, 0, len(nn)) + for _, n := range nn { + if !g.processed[NodeHash(n)] { + mm = append(mm, n) + } + } + return mm +} + // Children provides a list of children of the node n // // Child nodes are calculated on the fly based on node Relations() func (g *Graph) Children(n Node) []Node { if !g.invert { - return g.children(n) + return g.removeProcessed(g.children(n)) } - - return g.parents(n) + return g.removeProcessed(g.parents(n)) } func (g *Graph) children(n Node) []Node { @@ -128,22 +168,32 @@ func (g *Graph) children(n Node) []Node { return nn } +func (g *Graph) ChildrenA(n Node) []Node { + if !g.invert { + return g.children(n) + } + return g.parents(n) +} + // Parents provides a list of parents of the node n // // Parent nodes are calculated on the fly based on node Relations() func (g *Graph) Parents(n Node) []Node { if !g.invert { - return g.parents(n) + return g.removeProcessed(g.parents(n)) } - - return g.children(n) + return g.removeProcessed(g.children(n)) } func (g *Graph) parents(n Node) []Node { nn := make([]Node, 0) - for _, m := range g.Nodes { - if IDs, has := m.Relations()[n.Resource()]; has { + for _, m := range g.nodes { + r := m.Relations() + if r == nil { + continue + } + if IDs, has := r[n.Resource()]; has { for _, ID := range IDs { if n.ID() == ID { nn = append(nn, m) @@ -169,12 +219,29 @@ func (g *Graph) ParentsC(n Node) []Node { return mm } +func (g *Graph) ParentsA(n Node) []Node { + if !g.invert { + return g.parents(n) + } + return g.children(n) +} + // Validate performs a basic data validation over all the nodes. func (g *Graph) Validate() error { // @todo... return nil } +func (g *Graph) Nodes() []Node { + nn := make([]Node, 0, len(g.nodes)) + for _, n := range g.nodes { + if !g.processed[NodeHash(n)] { + nn = append(nn, n) + } + } + return nn +} + // Run invokes all operations while respecting relations (dependencies) and solving // dependency conflicts. // @@ -185,10 +252,10 @@ func (g *Graph) Validate() error { // // The order of above operations is a bit more complex, but the general flow is that. func (g *Graph) Run(ctx context.Context) error { - for len(g.Nodes) > 0 { + for len(g.nodes) > 0 { // Find all root nodes in the current graph state; those nodes are allowed to run. - nn := make([]Node, 0, len(g.Nodes)/2) - for _, n := range g.Nodes { + nn := make([]Node, 0, len(g.Nodes())/2) + for _, n := range g.Nodes() { // We should not take into account conflicted parent nodes, as they already resolved // the conflict. if len(g.ParentsC(n)) == 0 { @@ -216,11 +283,16 @@ func (g *Graph) Run(ctx context.Context) error { // are allowed to run. func (g *Graph) runRegular(ctx context.Context, nn []Node) error { for _, n := range nn { - err := n.Run(ctx) + err := n.Run(ctx, g.Children(n), g.ParentsA(n)) if err != nil { return err } + // When a child node is processed check if it's parent can be removed. + // A parent node can only be removed if all of it's child nodes have already been + // processed + g.MarkProcessed(n) + g.Remove(g.ParentsA(n)...) g.Remove(n) } @@ -240,14 +312,14 @@ func (g *Graph) runResolution(ctx context.Context) error { // For example: A -> B -> A -> c -> D; where A B C is the cycle and C is a branch from the cycle. // The code will works just fine, but it won't be that optimal so it should be improved to do // actual cycle detection. - for _, m := range g.Nodes { + for _, m := range g.Nodes() { if !g.conflicts[NodeHash(m)] { n = m break } } - err := n.ResolveConflict(ctx) + err := n.ResolveConflict(ctx, g.Children(n), g.Parents(n)) if err != nil { return err } diff --git a/pkg/envoy/graph_test.go b/pkg/envoy/graph_test.go index 7a34bc60b..81b272b57 100644 --- a/pkg/envoy/graph_test.go +++ b/pkg/envoy/graph_test.go @@ -12,7 +12,7 @@ var ( type ( NodeTest struct { Id string - relations NodeRelation + relations map[string][]string exOrder *[]string } ) @@ -25,16 +25,16 @@ func (n *NodeTest) Resource() string { return "test" } -func (n *NodeTest) Relations() NodeRelation { +func (n *NodeTest) Relations() map[string][]string { return n.relations } -func (n *NodeTest) Run(ctx context.Context) error { +func (n *NodeTest) Run(ctx context.Context, cc []Node, pp []Node) error { ops = append(ops, "R:"+n.ID()) return nil } -func (n *NodeTest) ResolveConflict(ctx context.Context) error { +func (n *NodeTest) ResolveConflict(ctx context.Context, cc []Node, pp []Node) error { ops = append(ops, "C:"+n.ID()) return nil } @@ -50,8 +50,8 @@ func TestGraphStructure_relations(t *testing.T) { { name: "N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2"}}}, + {Id: "N2", relations: map[string][]string{}}, }, node: "N1", children: []string{"N2"}, @@ -60,8 +60,8 @@ func TestGraphStructure_relations(t *testing.T) { { name: "N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2"}}}, + {Id: "N2", relations: map[string][]string{}}, }, node: "N2", children: []string{}, @@ -70,7 +70,7 @@ func TestGraphStructure_relations(t *testing.T) { { name: "N1 -> N1 :: cycle to self", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N1"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N1"}}}, }, node: "N1", children: []string{"N1"}, @@ -79,8 +79,8 @@ func TestGraphStructure_relations(t *testing.T) { { name: "N1 -> N1 -> N2 <- N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N1", "N2"}}}, - {Id: "N2", relations: NodeRelation{"test": []string{"N2"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N1", "N2"}}}, + {Id: "N2", relations: map[string][]string{"test": []string{"N2"}}}, }, node: "N1", children: []string{"N1", "N2"}, @@ -89,8 +89,8 @@ func TestGraphStructure_relations(t *testing.T) { { name: "N1 -> N1 -> N2 <- N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N1", "N2"}}}, - {Id: "N2", relations: NodeRelation{"test": []string{"N2"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N1", "N2"}}}, + {Id: "N2", relations: map[string][]string{"test": []string{"N2"}}}, }, node: "N2", children: []string{"N2"}, @@ -104,8 +104,8 @@ func TestGraphStructure_relations(t *testing.T) { g.Add(n) } - cc := g.Children(g.findNode("test", test.node)[0]) - pp := g.Parents(g.findNode("test", test.node)[0]) + cc := g.Children(g.FindNode("test", test.node)[0]) + pp := g.Parents(g.FindNode("test", test.node)[0]) if len(cc) != len(test.children) { t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc)) @@ -142,8 +142,8 @@ func TestGraphStructure_inversion(t *testing.T) { { name: "N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2"}}}, + {Id: "N2", relations: map[string][]string{}}, }, node: "N1", children: []string{}, @@ -152,8 +152,8 @@ func TestGraphStructure_inversion(t *testing.T) { { name: "N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2"}}}, + {Id: "N2", relations: map[string][]string{}}, }, node: "N2", children: []string{"N1"}, @@ -162,7 +162,7 @@ func TestGraphStructure_inversion(t *testing.T) { { name: "N1 -> N1 :: cycle to self", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N1"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N1"}}}, }, node: "N1", children: []string{"N1"}, @@ -178,8 +178,8 @@ func TestGraphStructure_inversion(t *testing.T) { g.Invert() - cc := g.Children(g.findNode("test", test.node)[0]) - pp := g.Parents(g.findNode("test", test.node)[0]) + cc := g.Children(g.FindNode("test", test.node)[0]) + pp := g.Parents(g.FindNode("test", test.node)[0]) if len(cc) != len(test.children) { t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc)) @@ -214,33 +214,40 @@ func TestGraphStructure_execution(t *testing.T) { { name: "N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2"}}}, + {Id: "N2", relations: map[string][]string{}}, }, ops: []string{"R:N2", "R:N1"}, }, + { + name: "N1 -> N1", + nodes: []*NodeTest{ + {Id: "N1", relations: map[string][]string{"test": []string{"N1"}}}, + }, + ops: []string{"C:N1", "R:N1"}, + }, { name: "N1 -> N1 -> N2", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2", "N1"}}}, - {Id: "N2", relations: NodeRelation{}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2", "N1"}}}, + {Id: "N2", relations: map[string][]string{}}, }, ops: []string{"R:N2", "C:N1", "R:N1"}, }, { name: "N2 -> N1 -> N1", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N1"}}}, - {Id: "N2", relations: NodeRelation{"test": []string{"N1"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N1"}}}, + {Id: "N2", relations: map[string][]string{"test": []string{"N1"}}}, }, ops: []string{"C:N1", "R:N1", "R:N2"}, }, { name: "N1 -> N1 <-> N2 <-> N3 <- N1", nodes: []*NodeTest{ - {Id: "N1", relations: NodeRelation{"test": []string{"N2", "N1", "N3"}}}, - {Id: "N2", relations: NodeRelation{"test": []string{"N1", "N3"}}}, - {Id: "N3", relations: NodeRelation{"test": []string{"N2"}}}, + {Id: "N1", relations: map[string][]string{"test": []string{"N2", "N1", "N3"}}}, + {Id: "N2", relations: map[string][]string{"test": []string{"N1", "N3"}}}, + {Id: "N3", relations: map[string][]string{"test": []string{"N2"}}}, }, ops: []string{"C:N1", "C:N2", "R:N3", "R:N1", "R:N2"}, }, @@ -253,6 +260,9 @@ func TestGraphStructure_execution(t *testing.T) { g.Add(n) } + // spew.Dump(g.nodes) + // spew.Dump("___", ops) + g.Invert() g.Run(context.Background()) @@ -261,27 +271,5 @@ func TestGraphStructure_execution(t *testing.T) { t.Errorf("[%s] operation missmatch; exp=%s got=%s", test.name, o, ops[i]) } } - - // if len(cc) != len(test.children) { - // t.Errorf("[%s] node child missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.children), len(cc)) - // return - // } - // for i, c := range cc { - // if c.ID() != test.children[i] { - // t.Errorf("[%s] node child missmatch; exp=%s got=%s pos=%d", test.name, test.children[i], c.ID(), i) - // return - // } - // } - - // if len(pp) != len(test.parents) { - // t.Errorf("[%s] node parent missmatch; list range doesnt match; exp=%d got=%d", test.name, len(test.parents), len(pp)) - // return - // } - // for i, p := range pp { - // if p.ID() != test.parents[i] { - // t.Errorf("[%s] node parent missmatch; exp=%s got=%s pos=%d", test.name, test.parents[i], p.ID(), i) - // return - // } - // } } }