From 89245fbdae4df56f0d6364d3d91afd9ac58edd1c Mon Sep 17 00:00:00 2001 From: lash Date: Fri, 30 Nov 2018 16:01:08 +0100 Subject: [PATCH 01/13] p2p/simulation: WIP minimal snapshot test --- p2p/simulations/network_test.go | 73 +++++++++++++++++++++ p2p/simulations/test.go | 2 +- swarm/network/simulation/simulation_test.go | 35 +++------- 3 files changed, 83 insertions(+), 27 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index f34935265127..accdda5b7d9c 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -22,10 +22,83 @@ import ( "testing" "time" + "github.com/ethereum/go-ethereum/log" + "github.com/ethereum/go-ethereum/node" "github.com/ethereum/go-ethereum/p2p/enode" "github.com/ethereum/go-ethereum/p2p/simulations/adapters" ) +func TestSnapshot(t *testing.T) { + protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) + adapter := adapters.NewSimAdapter(adapters.Services{ + "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { + svc := NewNoopService() + protoCMap[ctx.Config.ID] = svc.C + return svc, nil + }, + }) + network := NewNetwork(adapter, &NetworkConfig{ + DefaultService: "noopwoop", + }) + defer network.Shutdown() + + nodeCount := 20 + ids := make([]enode.ID, nodeCount) + for i := 0; i < nodeCount; i++ { + conf := adapters.RandomNodeConfig() + node, err := network.NewNodeWithConfig(conf) + if err != nil { + t.Fatalf("error creating node: %s", err) + } + if err := network.Start(node.ID()); err != nil { + t.Fatalf("error starting node: %s", err) + } + ids[i] = node.ID() + } + + go func() { + for i, id := range ids { + peerID := ids[(i+1)%len(ids)] + if err := network.Connect(id, peerID); err != nil { + t.Fatal(err) + } + } + }() + + evC := make(chan *Event) + sub := network.Events().Subscribe(evC) + defer sub.Unsubscribe() + + ctx, cancel := context.WithTimeout(context.TODO(), time.Second) + defer cancel() + + connEventCount := nodeCount +OUTER: + for { + select { + case <-ctx.Done(): + t.Fatal(ctx.Err()) + case ev := <-evC: + if ev.Type == EventTypeConn && !ev.Control { + connEventCount-- + log.Debug("ev", "count", connEventCount) + if connEventCount == 0 { + break OUTER + } + } + } + } + + for nodid, peers := range protoCMap { + for peerid, peerC := range peers { + log.Debug("getting ", "node", nodid, "peer", peerid) + <-peerC + } + } + + t.Logf("ok") +} + // TestNetworkSimulation creates a multi-node simulation network with each node // connected in a ring topology, checks that all nodes successfully handshake // with each other and that a snapshot fully represents the desired topology diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index beeb414e41b3..451c06080b26 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -23,7 +23,7 @@ func NewNoopService(ackC map[enode.ID]chan struct{}) *NoopService { func (t *NoopService) Protocols() []p2p.Protocol { return []p2p.Protocol{ - { + p2p.Protocol{ Name: "noop", Version: 666, Length: 0, diff --git a/swarm/network/simulation/simulation_test.go b/swarm/network/simulation/simulation_test.go index ca8599d7c0f4..4667a2abc329 100644 --- a/swarm/network/simulation/simulation_test.go +++ b/swarm/network/simulation/simulation_test.go @@ -26,9 +26,8 @@ import ( "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/node" - "github.com/ethereum/go-ethereum/p2p" + "github.com/ethereum/go-ethereum/p2p/simulations" "github.com/ethereum/go-ethereum/p2p/simulations/adapters" - "github.com/ethereum/go-ethereum/rpc" colorable "github.com/mattn/go-colorable" ) @@ -182,39 +181,23 @@ func noopServiceFunc(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, f return newNoopService(), nil, nil } -// noopService is the service that does not do anything -// but implements node.Service interface. -type noopService struct{} - func newNoopService() node.Service { return &noopService{} } -func (t *noopService) Protocols() []p2p.Protocol { - return []p2p.Protocol{} -} - -func (t *noopService) APIs() []rpc.API { - return []rpc.API{} -} - -func (t *noopService) Start(server *p2p.Server) error { - return nil -} - -func (t *noopService) Stop() error { - return nil -} - -// a helper function for most basic noop service -// of a different type then noopService to test +// a helper function for most basic Noop service +// of a different type then NoopService to test // multiple services on one node. func noopService2Func(ctx *adapters.ServiceContext, b *sync.Map) (node.Service, func(), error) { return new(noopService2), nil, nil } -// noopService2 is the service that does not do anything +// NoopService2 is the service that does not do anything // but implements node.Service interface. type noopService2 struct { - noopService + simulations.NoopService +} + +type noopService struct { + simulations.NoopService } From 1e0244d56d32c52948e83ec141da9c7aa3338bfa Mon Sep 17 00:00:00 2001 From: lash Date: Sun, 2 Dec 2018 12:35:06 +0100 Subject: [PATCH 02/13] p2p/simulation: Add snapshot create, load and verify to snapshot test --- p2p/simulations/network_test.go | 114 +++++++++++++++++++++++++++++++- 1 file changed, 111 insertions(+), 3 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index accdda5b7d9c..d84a8ff74778 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -18,6 +18,7 @@ package simulations import ( "context" + "encoding/json" "fmt" "testing" "time" @@ -28,11 +29,13 @@ import ( "github.com/ethereum/go-ethereum/p2p/simulations/adapters" ) +// Tests that a created snapshot with a minimal service only contains the expected connections +// and that a network when loaded with this snapshot only contains those same connections func TestSnapshot(t *testing.T) { protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - svc := NewNoopService() + svc := NewNoopService(false) protoCMap[ctx.Config.ID] = svc.C return svc, nil }, @@ -40,7 +43,14 @@ func TestSnapshot(t *testing.T) { network := NewNetwork(adapter, &NetworkConfig{ DefaultService: "noopwoop", }) - defer network.Shutdown() + + // \todo consider making a member of network, set to true threadsafe when shutdown + runningOne := true + defer func() { + if runningOne { + network.Shutdown() + } + }() nodeCount := 20 ids := make([]enode.ID, nodeCount) @@ -72,6 +82,7 @@ func TestSnapshot(t *testing.T) { ctx, cancel := context.WithTimeout(context.TODO(), time.Second) defer cancel() + checkIds := make(map[enode.ID][]enode.ID) connEventCount := nodeCount OUTER: for { @@ -80,6 +91,11 @@ OUTER: t.Fatal(ctx.Err()) case ev := <-evC: if ev.Type == EventTypeConn && !ev.Control { + if !ev.Conn.Up { + t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) + } + checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) + checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) connEventCount-- log.Debug("ev", "count", connEventCount) if connEventCount == 0 { @@ -89,14 +105,106 @@ OUTER: } } + snap, err := network.Snapshot() + if err != nil { + t.Fatal(err) + } + j, err := json.Marshal(snap) + if err != nil { + t.Fatal(err) + } + log.Debug("snapshot taken", "nodes", len(snap.Nodes), "conns", len(snap.Conns), "json", string(j)) + + // verify that the snap element numbers check out + if len(checkIds) != len(snap.Conns) || len(checkIds) != len(snap.Nodes) { + t.Fatalf("snapshot wrong node,conn counts %d,%d != %d", len(snap.Nodes), len(snap.Conns), len(checkIds)) + } + + // wait for all protocols to signal to close down for nodid, peers := range protoCMap { for peerid, peerC := range peers { log.Debug("getting ", "node", nodid, "peer", peerid) <-peerC } } + runningOne = false + sub.Unsubscribe() + network.Shutdown() + + // check that we have all expected connections in snapshot + for nodid, nodConns := range checkIds { + for _, nodConn := range nodConns { + var match bool + for _, snapConn := range snap.Conns { + if snapConn.One == nodid && snapConn.Other == nodConn { + match = true + break + } else if snapConn.Other == nodid && snapConn.One == nodConn { + match = true + break + } + } + if !match { + t.Fatalf("snapshot missing conn %v -> %v", nodid, nodConn) + } + } + } + log.Info("snapshot checked") + + // PART II + // load snapshot and verify that exactly same connections are formed + protoCMap = make(map[enode.ID]map[enode.ID]chan struct{}) + adapter = adapters.NewSimAdapter(adapters.Services{ + "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { + svc := NewNoopService(false) + protoCMap[ctx.Config.ID] = svc.C + return svc, nil + }, + }) + network = NewNetwork(adapter, &NetworkConfig{ + DefaultService: "noopwoop", + }) + defer func() { + network.Shutdown() + }() + + evC = make(chan *Event) + sub = network.Events().Subscribe(evC) + defer sub.Unsubscribe() + + go func() { + err = network.Load(snap) + if err != nil { + t.Fatal(err) + } + }() + + ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) + defer cancel() + + connEventCount = nodeCount +OUTER_TWO: + for { + select { + case <-ctx.Done(): + t.Fatal(ctx.Err()) + case ev := <-evC: + if ev.Type == EventTypeConn && !ev.Control { + if !ev.Conn.Up { + t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) + } + log.Debug("conn", "on", ev.Conn.One, "other", ev.Conn.Other) + checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) + checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) + connEventCount-- + log.Debug("ev", "count", connEventCount) + if connEventCount == 0 { + break OUTER_TWO + } + } + } + } - t.Logf("ok") } // TestNetworkSimulation creates a multi-node simulation network with each node From f44da20911634c7d1f5783a2ad5e89b203c234bd Mon Sep 17 00:00:00 2001 From: lash Date: Sun, 2 Dec 2018 12:50:09 +0100 Subject: [PATCH 03/13] build: add test tag for tests --- build/ci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/ci.go b/build/ci.go index 1bbc944714dd..6af3b78fdaf2 100644 --- a/build/ci.go +++ b/build/ci.go @@ -334,7 +334,7 @@ func doTest(cmdline []string) { // Test a single package at a time. CI builders are slow // and some tests run into timeouts under load. gotest := goTool("test", buildFlags(env)...) - gotest.Args = append(gotest.Args, "-p", "1", "-timeout", "5m") + gotest.Args = append(gotest.Args, "-p", "1", "-timeout", "5m", "-tags", "test") if *coverage { gotest.Args = append(gotest.Args, "-covermode=atomic", "-cover") } From 57522410d9b3f8c5f89f1a4ed0383c693ed9d7bb Mon Sep 17 00:00:00 2001 From: lash Date: Sun, 2 Dec 2018 13:10:34 +0100 Subject: [PATCH 04/13] p2p/simulations, build: Revert travis change, build test sym always --- build/ci.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/build/ci.go b/build/ci.go index 6af3b78fdaf2..1bbc944714dd 100644 --- a/build/ci.go +++ b/build/ci.go @@ -334,7 +334,7 @@ func doTest(cmdline []string) { // Test a single package at a time. CI builders are slow // and some tests run into timeouts under load. gotest := goTool("test", buildFlags(env)...) - gotest.Args = append(gotest.Args, "-p", "1", "-timeout", "5m", "-tags", "test") + gotest.Args = append(gotest.Args, "-p", "1", "-timeout", "5m") if *coverage { gotest.Args = append(gotest.Args, "-covermode=atomic", "-cover") } From de133fceeb70a93e31fb4650f2b8ffa441659f8d Mon Sep 17 00:00:00 2001 From: lash Date: Sun, 2 Dec 2018 13:52:20 +0100 Subject: [PATCH 05/13] p2p/simulations: Add comments, timeout check on additional events --- p2p/simulations/network_test.go | 95 +++++++++++++++++++++++++-------- 1 file changed, 74 insertions(+), 21 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index d84a8ff74778..337010109dad 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -32,18 +32,25 @@ import ( // Tests that a created snapshot with a minimal service only contains the expected connections // and that a network when loaded with this snapshot only contains those same connections func TestSnapshot(t *testing.T) { - protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) + + // PART I + // create snapshot from ring network + + // this is a minimal service, whose protocol will take exactly one message OR close of connection before quitting + //protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - svc := NewNoopService(false) - protoCMap[ctx.Config.ID] = svc.C - return svc, nil + // svc := NewNoopService(false) + // protoCMap[ctx.Config.ID] = svc.C + // return svc, nil + return NewNoopService(false), nil }, }) + + // create network network := NewNetwork(adapter, &NetworkConfig{ DefaultService: "noopwoop", }) - // \todo consider making a member of network, set to true threadsafe when shutdown runningOne := true defer func() { @@ -52,6 +59,7 @@ func TestSnapshot(t *testing.T) { } }() + // create and start nodes nodeCount := 20 ids := make([]enode.ID, nodeCount) for i := 0; i < nodeCount; i++ { @@ -66,6 +74,13 @@ func TestSnapshot(t *testing.T) { ids[i] = node.ID() } + // subscribe to peer events + evC := make(chan *Event) + sub := network.Events().Subscribe(evC) + defer sub.Unsubscribe() + + // connect nodes in a ring + // spawn separate thread to avoid deadlock in the event listeners go func() { for i, id := range ids { peerID := ids[(i+1)%len(ids)] @@ -75,13 +90,9 @@ func TestSnapshot(t *testing.T) { } }() - evC := make(chan *Event) - sub := network.Events().Subscribe(evC) - defer sub.Unsubscribe() - + // collect connection events up to expected number ctx, cancel := context.WithTimeout(context.TODO(), time.Second) defer cancel() - checkIds := make(map[enode.ID][]enode.ID) connEventCount := nodeCount OUTER: @@ -91,6 +102,8 @@ OUTER: t.Fatal(ctx.Err()) case ev := <-evC: if ev.Type == EventTypeConn && !ev.Control { + + // fail on any disconnect if !ev.Conn.Up { t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) } @@ -105,6 +118,7 @@ OUTER: } } + // create snapshot of current network snap, err := network.Snapshot() if err != nil { t.Fatal(err) @@ -121,17 +135,19 @@ OUTER: } // wait for all protocols to signal to close down - for nodid, peers := range protoCMap { - for peerid, peerC := range peers { - log.Debug("getting ", "node", nodid, "peer", peerid) - <-peerC - } - } + // for nodid, peers := range protoCMap { + // for peerid, peerC := range peers { + // log.Debug("getting ", "node", nodid, "peer", peerid) + // <-peerC + // } + // } + + // shut down sim network runningOne = false sub.Unsubscribe() network.Shutdown() - // check that we have all expected connections in snapshot + // check that we have all the expected connections in the snapshot for nodid, nodConns := range checkIds { for _, nodConn := range nodConns { var match bool @@ -153,12 +169,14 @@ OUTER: // PART II // load snapshot and verify that exactly same connections are formed - protoCMap = make(map[enode.ID]map[enode.ID]chan struct{}) + + //protoCMap = make(map[enode.ID]map[enode.ID]chan struct{}) adapter = adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - svc := NewNoopService(false) - protoCMap[ctx.Config.ID] = svc.C - return svc, nil + // svc := NewNoopService(false) + // protoCMap[ctx.Config.ID] = svc.C + // return svc, nil + return NewNoopService(false), nil }, }) network = NewNetwork(adapter, &NetworkConfig{ @@ -168,10 +186,13 @@ OUTER: network.Shutdown() }() + // subscribe to peer events evC = make(chan *Event) sub = network.Events().Subscribe(evC) defer sub.Unsubscribe() + // load the snapshot + // spawn separate thread to avoid deadlock in the event listeners go func() { err = network.Load(snap) if err != nil { @@ -179,6 +200,7 @@ OUTER: } }() + // collect connection events up to expected number ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) defer cancel() @@ -190,6 +212,8 @@ OUTER_TWO: t.Fatal(ctx.Err()) case ev := <-evC: if ev.Type == EventTypeConn && !ev.Control { + + // fail on any disconnect if !ev.Conn.Up { t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) } @@ -205,6 +229,35 @@ OUTER_TWO: } } + // check that we have all expected connections in the network + for _, snapConn := range snap.Conns { + var match bool + for nodid, nodConns := range checkIds { + for _, nodConn := range nodConns { + if snapConn.One == nodid && snapConn.Other == nodConn { + match = true + break + } else if snapConn.Other == nodid && snapConn.One == nodConn { + match = true + break + } + } + } + if !match { + t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) + } + } + + // verify that network didn't generate any other additional connection events after the ones we have collected within a reasonable period of time + ctx, cancel = context.WithTimeout(context.TODO(), time.Second) + defer cancel() + select { + case <-ctx.Done(): + case ev := <-evC: + if ev.Type == EventTypeConn { + t.Fatalf("Superfluous conn found %v -> %v", ev.Conn.One, ev.Conn.Other) + } + } } // TestNetworkSimulation creates a multi-node simulation network with each node From 8386370d0e5be961c6f6947c6846d12a4dca6c1f Mon Sep 17 00:00:00 2001 From: lash Date: Mon, 3 Dec 2018 14:42:33 +0100 Subject: [PATCH 06/13] p2p/simulation: Add benchmark template for minimal peer protocol init --- p2p/simulations/network_test.go | 78 +++++++++++++++++++++++++++++++++ 1 file changed, 78 insertions(+) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 337010109dad..5e81b83c91ca 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -20,6 +20,8 @@ import ( "context" "encoding/json" "fmt" + "strconv" + "strings" "testing" "time" @@ -392,3 +394,79 @@ func triggerChecks(ctx context.Context, ids []enode.ID, trigger chan enode.ID, i } } } + +// \todo: refactor to implement shapshots +// and connect configuration methods once these are moved from +// swarm/network/simulations/connect.go +func BenchmarkMinimalService(b *testing.B) { + b.Run("ring/32", benchmarkMinimalServiceTmp) +} + +func benchmarkMinimalServiceTmp(b *testing.B) { + + // stop timer to discard setup time pollution + b.StopTimer() + args := strings.Split(b.Name(), "/") + nodeCount, err := strconv.ParseInt(args[2], 10, 16) + if err != nil { + b.Fatal(err) + } + + for i := 0; i < b.N; i++ { + // this is a minimal service, whose protocol will close a channel upon run of protocol + // making it possible to bench the time it takes for the service to start and protocol actually to be run + protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) + adapter := adapters.NewSimAdapter(adapters.Services{ + "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { + svc := NewNoopService(true) + protoCMap[ctx.Config.ID] = svc.C + return svc, nil + }, + }) + + // create network + network := NewNetwork(adapter, &NetworkConfig{ + DefaultService: "noopwoop", + }) + defer network.Shutdown() + + // create and start nodes + ids := make([]enode.ID, nodeCount) + for i := 0; i < int(nodeCount); i++ { + conf := adapters.RandomNodeConfig() + node, err := network.NewNodeWithConfig(conf) + if err != nil { + b.Fatalf("error creating node: %s", err) + } + if err := network.Start(node.ID()); err != nil { + b.Fatalf("error starting node: %s", err) + } + ids[i] = node.ID() + } + + // ready, set, go + b.StartTimer() + + // connect nodes in a ring + for i, id := range ids { + peerID := ids[(i+1)%len(ids)] + if err := network.Connect(id, peerID); err != nil { + b.Fatal(err) + } + } + + // wait for all protocols to signal to close down + ctx, cancel := context.WithTimeout(context.TODO(), time.Second) + defer cancel() + for nodid, peers := range protoCMap { + for peerid, peerC := range peers { + log.Debug("getting ", "node", nodid, "peer", peerid) + select { + case <-ctx.Done(): + b.Fatal(ctx.Err()) + case <-peerC: + } + } + } + } +} From 953273480ca66772d226ae4766b996bf5e2f17a2 Mon Sep 17 00:00:00 2001 From: lash Date: Mon, 3 Dec 2018 15:10:46 +0100 Subject: [PATCH 07/13] p2p/simulations: Remove unused code --- p2p/simulations/network_test.go | 16 ---------------- 1 file changed, 16 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 5e81b83c91ca..3185a7ea6f63 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -39,12 +39,8 @@ func TestSnapshot(t *testing.T) { // create snapshot from ring network // this is a minimal service, whose protocol will take exactly one message OR close of connection before quitting - //protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - // svc := NewNoopService(false) - // protoCMap[ctx.Config.ID] = svc.C - // return svc, nil return NewNoopService(false), nil }, }) @@ -136,14 +132,6 @@ OUTER: t.Fatalf("snapshot wrong node,conn counts %d,%d != %d", len(snap.Nodes), len(snap.Conns), len(checkIds)) } - // wait for all protocols to signal to close down - // for nodid, peers := range protoCMap { - // for peerid, peerC := range peers { - // log.Debug("getting ", "node", nodid, "peer", peerid) - // <-peerC - // } - // } - // shut down sim network runningOne = false sub.Unsubscribe() @@ -172,12 +160,8 @@ OUTER: // PART II // load snapshot and verify that exactly same connections are formed - //protoCMap = make(map[enode.ID]map[enode.ID]chan struct{}) adapter = adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - // svc := NewNoopService(false) - // protoCMap[ctx.Config.ID] = svc.C - // return svc, nil return NewNoopService(false), nil }, }) From f32748a965d6e8128be0ec00f8c7f9a17f63243b Mon Sep 17 00:00:00 2001 From: lash Date: Mon, 3 Dec 2018 18:51:23 +0100 Subject: [PATCH 08/13] p2p/simulation: Correct timer reset --- p2p/simulations/network_test.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 3185a7ea6f63..5733bfd88578 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -389,7 +389,6 @@ func BenchmarkMinimalService(b *testing.B) { func benchmarkMinimalServiceTmp(b *testing.B) { // stop timer to discard setup time pollution - b.StopTimer() args := strings.Split(b.Name(), "/") nodeCount, err := strconv.ParseInt(args[2], 10, 16) if err != nil { @@ -429,7 +428,7 @@ func benchmarkMinimalServiceTmp(b *testing.B) { } // ready, set, go - b.StartTimer() + b.ResetTimer() // connect nodes in a ring for i, id := range ids { From fb82f708439ab0ce0127ff7b00de181984d632ca Mon Sep 17 00:00:00 2001 From: lash Date: Mon, 10 Dec 2018 14:10:26 +0100 Subject: [PATCH 09/13] p2p/simulations: Put snapshot check events in buffer and call blocking --- p2p/simulations/network_test.go | 23 ++++++++++++----------- p2p/simulations/test.go | 2 +- 2 files changed, 13 insertions(+), 12 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 5733bfd88578..71594c5cb3ec 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -41,7 +41,7 @@ func TestSnapshot(t *testing.T) { // this is a minimal service, whose protocol will take exactly one message OR close of connection before quitting adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - return NewNoopService(false), nil + return NewNoopService(nil), nil }, }) @@ -162,7 +162,7 @@ OUTER: adapter = adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - return NewNoopService(false), nil + return NewNoopService(nil), nil }, }) network = NewNetwork(adapter, &NetworkConfig{ @@ -173,24 +173,25 @@ OUTER: }() // subscribe to peer events - evC = make(chan *Event) + // every node up and conn up event will generate one additional control event + // therefore multiply the count by two + evC = make(chan *Event, (len(snap.Conns)*2)+(len(snap.Nodes)*2)) sub = network.Events().Subscribe(evC) defer sub.Unsubscribe() // load the snapshot // spawn separate thread to avoid deadlock in the event listeners - go func() { - err = network.Load(snap) - if err != nil { - t.Fatal(err) - } - }() + err = network.Load(snap) + if err != nil { + t.Fatal(err) + } // collect connection events up to expected number ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) defer cancel() connEventCount = nodeCount + OUTER_TWO: for { select { @@ -401,8 +402,8 @@ func benchmarkMinimalServiceTmp(b *testing.B) { protoCMap := make(map[enode.ID]map[enode.ID]chan struct{}) adapter := adapters.NewSimAdapter(adapters.Services{ "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { - svc := NewNoopService(true) - protoCMap[ctx.Config.ID] = svc.C + protoCMap[ctx.Config.ID] = make(map[enode.ID]chan struct{}) + svc := NewNoopService(protoCMap[ctx.Config.ID]) return svc, nil }, }) diff --git a/p2p/simulations/test.go b/p2p/simulations/test.go index 451c06080b26..beeb414e41b3 100644 --- a/p2p/simulations/test.go +++ b/p2p/simulations/test.go @@ -23,7 +23,7 @@ func NewNoopService(ackC map[enode.ID]chan struct{}) *NoopService { func (t *NoopService) Protocols() []p2p.Protocol { return []p2p.Protocol{ - p2p.Protocol{ + { Name: "noop", Version: 666, Length: 0, From b503e421dd906c8ca257a685e99de895b78cc129 Mon Sep 17 00:00:00 2001 From: Janos Guljas Date: Mon, 10 Dec 2018 15:08:39 +0100 Subject: [PATCH 10/13] p2p/simulations: TestSnapshot fail if Load function returns early --- p2p/simulations/network_test.go | 107 ++++++++++++++++++-------------- 1 file changed, 60 insertions(+), 47 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 71594c5cb3ec..7476f1730061 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -175,65 +175,78 @@ OUTER: // subscribe to peer events // every node up and conn up event will generate one additional control event // therefore multiply the count by two - evC = make(chan *Event, (len(snap.Conns)*2)+(len(snap.Nodes)*2)) + evC = make(chan *Event) sub = network.Events().Subscribe(evC) defer sub.Unsubscribe() - // load the snapshot - // spawn separate thread to avoid deadlock in the event listeners - err = network.Load(snap) - if err != nil { - t.Fatal(err) - } + // Channel that is signalling when the Load function returns + // to fail the test in the event for loop if that happens + // before all connection events are counted. + loadDoneC := make(chan struct{}) - // collect connection events up to expected number - ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) - defer cancel() - - connEventCount = nodeCount - -OUTER_TWO: - for { - select { - case <-ctx.Done(): - t.Fatal(ctx.Err()) - case ev := <-evC: - if ev.Type == EventTypeConn && !ev.Control { + go func() { + // collect connection events up to expected number + ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) + defer cancel() - // fail on any disconnect - if !ev.Conn.Up { - t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) - } - log.Debug("conn", "on", ev.Conn.One, "other", ev.Conn.Other) - checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) - checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) - connEventCount-- - log.Debug("ev", "count", connEventCount) - if connEventCount == 0 { - break OUTER_TWO + connEventCount = nodeCount + + OUTER_TWO: + for { + select { + case <-ctx.Done(): + t.Fatal(ctx.Err()) + case ev := <-evC: + if ev.Type == EventTypeConn && !ev.Control { + + // fail on any disconnect + if !ev.Conn.Up { + t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) + } + log.Debug("conn", "on", ev.Conn.One, "other", ev.Conn.Other) + checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) + checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) + connEventCount-- + log.Debug("ev", "count", connEventCount) + if connEventCount == 0 { + break OUTER_TWO + } } + case <-loadDoneC: + // if load function returns before all expected events are caught. + // fail the test + t.Fatal("load function returned before all connections are established") } } - } - // check that we have all expected connections in the network - for _, snapConn := range snap.Conns { - var match bool - for nodid, nodConns := range checkIds { - for _, nodConn := range nodConns { - if snapConn.One == nodid && snapConn.Other == nodConn { - match = true - break - } else if snapConn.Other == nodid && snapConn.One == nodConn { - match = true - break + // check that we have all expected connections in the network + for _, snapConn := range snap.Conns { + var match bool + for nodid, nodConns := range checkIds { + for _, nodConn := range nodConns { + if snapConn.One == nodid && snapConn.Other == nodConn { + match = true + break + } else if snapConn.Other == nodid && snapConn.One == nodConn { + match = true + break + } } } + if !match { + t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) + } } - if !match { - t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) - } + }() + + // load the snapshot + // spawn separate thread to avoid deadlock in the event listeners + err = network.Load(snap) + if err != nil { + t.Fatal(err) } + // signal the event for event loop that Load function has returned + close(loadDoneC) // verify that network didn't generate any other additional connection events after the ones we have collected within a reasonable period of time ctx, cancel = context.WithTimeout(context.TODO(), time.Second) @@ -241,7 +254,7 @@ OUTER_TWO: select { case <-ctx.Done(): case ev := <-evC: - if ev.Type == EventTypeConn { + if ev.Type == EventTypeConn && !ev.Control { t.Fatalf("Superfluous conn found %v -> %v", ev.Conn.One, ev.Conn.Other) } } From 76c6758b0089010c43b9a78bf9bbe985be6e2658 Mon Sep 17 00:00:00 2001 From: Janos Guljas Date: Mon, 10 Dec 2018 16:24:00 +0100 Subject: [PATCH 11/13] p2p/simulations: TestSnapshot wait for all connections before returning --- p2p/simulations/network_test.go | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 7476f1730061..209960caa892 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -183,6 +183,8 @@ OUTER: // to fail the test in the event for loop if that happens // before all connection events are counted. loadDoneC := make(chan struct{}) + // Channel that signals when all connections are established. + connsC := make(chan struct{}) go func() { // collect connection events up to expected number @@ -237,6 +239,9 @@ OUTER: t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) } } + + // close the channel to signal that all connections are established + close(connsC) }() // load the snapshot @@ -248,6 +253,13 @@ OUTER: // signal the event for event loop that Load function has returned close(loadDoneC) + // wait for all connections + select { + case <-connsC: + case <-time.After(10 * time.Second): + t.Fatal("timing out waiting for connections") + } + // verify that network didn't generate any other additional connection events after the ones we have collected within a reasonable period of time ctx, cancel = context.WithTimeout(context.TODO(), time.Second) defer cancel() From 834bafd0101590810a1d8191ccfbe51866bf0c0f Mon Sep 17 00:00:00 2001 From: lash Date: Wed, 12 Dec 2018 14:54:10 +0100 Subject: [PATCH 12/13] p2p/simulation: Revert to before wait for snap load (5e75594) --- p2p/simulations/network_test.go | 119 +++++++++++++------------------- 1 file changed, 47 insertions(+), 72 deletions(-) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 209960caa892..71594c5cb3ec 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -175,89 +175,64 @@ OUTER: // subscribe to peer events // every node up and conn up event will generate one additional control event // therefore multiply the count by two - evC = make(chan *Event) + evC = make(chan *Event, (len(snap.Conns)*2)+(len(snap.Nodes)*2)) sub = network.Events().Subscribe(evC) defer sub.Unsubscribe() - // Channel that is signalling when the Load function returns - // to fail the test in the event for loop if that happens - // before all connection events are counted. - loadDoneC := make(chan struct{}) - // Channel that signals when all connections are established. - connsC := make(chan struct{}) + // load the snapshot + // spawn separate thread to avoid deadlock in the event listeners + err = network.Load(snap) + if err != nil { + t.Fatal(err) + } - go func() { - // collect connection events up to expected number - ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) - defer cancel() + // collect connection events up to expected number + ctx, cancel = context.WithTimeout(context.TODO(), time.Second*3) + defer cancel() - connEventCount = nodeCount - - OUTER_TWO: - for { - select { - case <-ctx.Done(): - t.Fatal(ctx.Err()) - case ev := <-evC: - if ev.Type == EventTypeConn && !ev.Control { - - // fail on any disconnect - if !ev.Conn.Up { - t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) - } - log.Debug("conn", "on", ev.Conn.One, "other", ev.Conn.Other) - checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) - checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) - connEventCount-- - log.Debug("ev", "count", connEventCount) - if connEventCount == 0 { - break OUTER_TWO - } + connEventCount = nodeCount + +OUTER_TWO: + for { + select { + case <-ctx.Done(): + t.Fatal(ctx.Err()) + case ev := <-evC: + if ev.Type == EventTypeConn && !ev.Control { + + // fail on any disconnect + if !ev.Conn.Up { + t.Fatalf("unexpected disconnect: %v -> %v", ev.Conn.One, ev.Conn.Other) + } + log.Debug("conn", "on", ev.Conn.One, "other", ev.Conn.Other) + checkIds[ev.Conn.One] = append(checkIds[ev.Conn.One], ev.Conn.Other) + checkIds[ev.Conn.Other] = append(checkIds[ev.Conn.Other], ev.Conn.One) + connEventCount-- + log.Debug("ev", "count", connEventCount) + if connEventCount == 0 { + break OUTER_TWO } - case <-loadDoneC: - // if load function returns before all expected events are caught. - // fail the test - t.Fatal("load function returned before all connections are established") } } + } - // check that we have all expected connections in the network - for _, snapConn := range snap.Conns { - var match bool - for nodid, nodConns := range checkIds { - for _, nodConn := range nodConns { - if snapConn.One == nodid && snapConn.Other == nodConn { - match = true - break - } else if snapConn.Other == nodid && snapConn.One == nodConn { - match = true - break - } + // check that we have all expected connections in the network + for _, snapConn := range snap.Conns { + var match bool + for nodid, nodConns := range checkIds { + for _, nodConn := range nodConns { + if snapConn.One == nodid && snapConn.Other == nodConn { + match = true + break + } else if snapConn.Other == nodid && snapConn.One == nodConn { + match = true + break } } - if !match { - t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) - } } - - // close the channel to signal that all connections are established - close(connsC) - }() - - // load the snapshot - // spawn separate thread to avoid deadlock in the event listeners - err = network.Load(snap) - if err != nil { - t.Fatal(err) - } - // signal the event for event loop that Load function has returned - close(loadDoneC) - - // wait for all connections - select { - case <-connsC: - case <-time.After(10 * time.Second): - t.Fatal("timing out waiting for connections") + if !match { + t.Fatalf("network missing conn %v -> %v", snapConn.One, snapConn.Other) + } } // verify that network didn't generate any other additional connection events after the ones we have collected within a reasonable period of time @@ -266,7 +241,7 @@ OUTER: select { case <-ctx.Done(): case ev := <-evC: - if ev.Type == EventTypeConn && !ev.Control { + if ev.Type == EventTypeConn { t.Fatalf("Superfluous conn found %v -> %v", ev.Conn.One, ev.Conn.Other) } } From a0dfb36342b9e7883b3ceec0d6a415ce346d9bb7 Mon Sep 17 00:00:00 2001 From: Janos Guljas Date: Wed, 12 Dec 2018 15:14:06 +0100 Subject: [PATCH 13/13] p2p/simulations: add "conns after load" subtest to TestSnapshot and nudge --- p2p/simulations/network_test.go | 31 +++++++++++++++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/p2p/simulations/network_test.go b/p2p/simulations/network_test.go index 71594c5cb3ec..b7852addb986 100644 --- a/p2p/simulations/network_test.go +++ b/p2p/simulations/network_test.go @@ -245,6 +245,37 @@ OUTER_TWO: t.Fatalf("Superfluous conn found %v -> %v", ev.Conn.One, ev.Conn.Other) } } + + // This test validates if all connections from the snapshot + // are created in the network. + t.Run("conns after load", func(t *testing.T) { + // Create new network. + n := NewNetwork( + adapters.NewSimAdapter(adapters.Services{ + "noopwoop": func(ctx *adapters.ServiceContext) (node.Service, error) { + return NewNoopService(nil), nil + }, + }), + &NetworkConfig{ + DefaultService: "noopwoop", + }, + ) + defer n.Shutdown() + + // Load the same snapshot. + err := n.Load(snap) + if err != nil { + t.Fatal(err) + } + + // Check every connection from the snapshot + // if it is in the network, too. + for _, c := range snap.Conns { + if n.GetConn(c.One, c.Other) == nil { + t.Errorf("missing connection: %s -> %s", c.One, c.Other) + } + } + }) } // TestNetworkSimulation creates a multi-node simulation network with each node