From 26b1dcedd4301e04abf6e137ceb3f89cbec6778b Mon Sep 17 00:00:00 2001 From: Tim Wu Date: Mon, 30 May 2022 21:30:44 -0400 Subject: [PATCH 1/5] set channel to nil on close, check for nil chan --- dot/peerset/peerset.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index df19ec2d51..b9fa59c395 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -690,10 +690,12 @@ func (ps *PeerSet) disconnect(setIdx int, reason DropReason, peers ...peer.ID) e return fmt.Errorf("cannot disconnect: %w", err) } - ps.resultMsgCh <- Message{ - Status: Drop, - setID: uint64(setIdx), - PeerID: pid, + if ps.resultMsgCh != nil { + ps.resultMsgCh <- Message{ + Status: Drop, + setID: uint64(setIdx), + PeerID: pid, + } } // TODO: figure out the condition of connection refuse. @@ -765,6 +767,7 @@ func (ps *PeerSet) periodicallyAllocateSlots(ctx context.Context) { defer func() { ticker.Stop() close(ps.resultMsgCh) + ps.resultMsgCh = nil }() for { From 14dc1e551e33ea3f40f2b5cc7d7825a65a4678ad Mon Sep 17 00:00:00 2001 From: Tim Wu Date: Wed, 1 Jun 2022 15:43:40 -0400 Subject: [PATCH 2/5] refactor parallel goroutines to single goroutine --- dot/peerset/peerset.go | 52 +++++++++++++++--------------------------- 1 file changed, 19 insertions(+), 33 deletions(-) diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index b9fa59c395..3f9229493a 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -690,12 +690,10 @@ func (ps *PeerSet) disconnect(setIdx int, reason DropReason, peers ...peer.ID) e return fmt.Errorf("cannot disconnect: %w", err) } - if ps.resultMsgCh != nil { - ps.resultMsgCh <- Message{ - Status: Drop, - setID: uint64(setIdx), - PeerID: pid, - } + ps.resultMsgCh <- Message{ + Status: Drop, + setID: uint64(setIdx), + PeerID: pid, } // TODO: figure out the condition of connection refuse. @@ -714,15 +712,27 @@ func (ps *PeerSet) start(ctx context.Context, actionQueue chan action) { ps.actionQueue = actionQueue ps.resultMsgCh = make(chan Message, msgChanSize) - go ps.listenAction(ctx) - go ps.periodicallyAllocateSlots(ctx) + go ps.listenActionAllocSlots(ctx) } -func (ps *PeerSet) listenAction(ctx context.Context) { +func (ps *PeerSet) listenActionAllocSlots(ctx context.Context) { + ticker := time.NewTicker(ps.nextPeriodicAllocSlots) + + defer func() { + ticker.Stop() + close(ps.resultMsgCh) + }() + for { select { case <-ctx.Done(): return + case <-ticker.C: + for setID := 0; setID < ps.peerState.getSetLength(); setID++ { + if err := ps.allocSlots(setID); err != nil { + logger.Warnf("failed to allocate slots: %s", err) + } + } case act, ok := <-ps.actionQueue: if !ok { return @@ -760,27 +770,3 @@ func (ps *PeerSet) listenAction(ctx context.Context) { } } } - -func (ps *PeerSet) periodicallyAllocateSlots(ctx context.Context) { - ticker := time.NewTicker(ps.nextPeriodicAllocSlots) - - defer func() { - ticker.Stop() - close(ps.resultMsgCh) - ps.resultMsgCh = nil - }() - - for { - select { - case <-ctx.Done(): - logger.Debugf("peerset slot allocation exiting: %s", ctx.Err()) - return - case <-ticker.C: - for setID := 0; setID < ps.peerState.getSetLength(); setID++ { - if err := ps.allocSlots(setID); err != nil { - logger.Warnf("failed to allocate slots: %s", err) - } - } - } - } -} From dd87ef1f457d9985021cb1e1d69bef0439295ef8 Mon Sep 17 00:00:00 2001 From: Tim Wu Date: Wed, 8 Jun 2022 15:30:44 -0400 Subject: [PATCH 3/5] cr feedback --- dot/peerset/peerset.go | 1 + 1 file changed, 1 insertion(+) diff --git a/dot/peerset/peerset.go b/dot/peerset/peerset.go index 3f9229493a..dea6956f90 100644 --- a/dot/peerset/peerset.go +++ b/dot/peerset/peerset.go @@ -726,6 +726,7 @@ func (ps *PeerSet) listenActionAllocSlots(ctx context.Context) { for { select { case <-ctx.Done(): + logger.Debugf("peerset slot allocation exiting: %s", ctx.Err()) return case <-ticker.C: for setID := 0; setID < ps.peerState.getSetLength(); setID++ { From 2fb93bace81c24c9412c31ca58fc659420361bb8 Mon Sep 17 00:00:00 2001 From: Tim Wu Date: Thu, 9 Jun 2022 20:54:49 -0400 Subject: [PATCH 4/5] bump to 45m --- .github/workflows/unit-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/unit-tests.yml b/.github/workflows/unit-tests.yml index 9924289545..0132ee7cc0 100644 --- a/.github/workflows/unit-tests.yml +++ b/.github/workflows/unit-tests.yml @@ -67,7 +67,7 @@ jobs: echo "$HOME/.local/bin" >> $GITHUB_PATH - name: Run unit tests - run: go test -short -coverprofile=coverage.out -covermode=atomic -timeout=30m ./... + run: go test -short -coverprofile=coverage.out -covermode=atomic -timeout=45m ./... - name: Test State - Race run: make test-state-race From 6c62a28d0e529eb62084a91bfcec08c6310facf8 Mon Sep 17 00:00:00 2001 From: Tim Wu Date: Fri, 10 Jun 2022 10:05:35 -0400 Subject: [PATCH 5/5] bump up integration to 45m --- .github/workflows/integration-tests.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/integration-tests.yml b/.github/workflows/integration-tests.yml index 80e138ef09..674099c7c4 100644 --- a/.github/workflows/integration-tests.yml +++ b/.github/workflows/integration-tests.yml @@ -60,4 +60,4 @@ jobs: restore-keys: ${{ runner.os }}-go-mod - name: Run integration tests - run: go test -timeout=30m -tags integration ${{ matrix.packages }} + run: go test -timeout=45m -tags integration ${{ matrix.packages }}