From 46ceaf8cf60100a4366ab2d2cdb6c308a0d67fd9 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 3 Apr 2019 16:43:31 -0700 Subject: [PATCH 1/2] discovery: only replace stale active syncer if disconnected In this commit, we address a bug where we'd attempt to replace the stale active syncer when it transitioned to a passive syncer. This replacement logic is only intended to happen when the active syncer disconnects, as rotateActiveSyncerCandidate chooses and queues its own replacement. --- discovery/sync_manager.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/discovery/sync_manager.go b/discovery/sync_manager.go index 12e3e4a25..43d95a163 100644 --- a/discovery/sync_manager.go +++ b/discovery/sync_manager.go @@ -353,19 +353,20 @@ func (m *SyncManager) roundRobinHandler() { // the set of inactive syncers. if staleActiveSyncer.transitioned { m.inactiveSyncers[s.cfg.peerPub] = s + } else { + // Otherwise, since the peer is disconnecting, + // we'll attempt to find a passive syncer that + // can replace it. + newActiveSyncer := m.chooseRandomSyncer(nil, false) + if newActiveSyncer != nil { + m.queueActiveSyncer(newActiveSyncer) + } } // Remove the internal active syncer references for this // peer. delete(m.pendingActiveSyncers, s.cfg.peerPub) delete(m.activeSyncers, s.cfg.peerPub) - - // We'll then attempt to find a passive syncer that can - // replace the stale active syncer. - newActiveSyncer := m.chooseRandomSyncer(nil, false) - if newActiveSyncer != nil { - m.queueActiveSyncer(newActiveSyncer) - } m.Unlock() // Signal to the caller that they can now proceed since From 00338c5ec2a8cd3d26eaab4cc5ec08bf1f8f4c1a Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Wed, 3 Apr 2019 19:11:03 -0700 Subject: [PATCH 2/2] discovery: properly handle SyncManager shutdown signal --- discovery/sync_manager.go | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/discovery/sync_manager.go b/discovery/sync_manager.go index 43d95a163..11d147a89 100644 --- a/discovery/sync_manager.go +++ b/discovery/sync_manager.go @@ -286,6 +286,13 @@ func (m *SyncManager) roundRobinHandler() { current = m.nextPendingActiveSyncer() m.Unlock() for current != nil { + // Ensure we properly handle a shutdown signal. + select { + case <-m.quit: + return + default: + } + // We'll avoid performing the transition with the lock // as it can potentially stall the SyncManager due to // the syncTransitionTimeout. @@ -531,6 +538,13 @@ func (m *SyncManager) forceHistoricalSync() { candidatesChosen := make(map[routing.Vertex]struct{}) s := m.chooseRandomSyncer(candidatesChosen, true) for s != nil { + // Ensure we properly handle a shutdown signal. + select { + case <-m.quit: + return + default: + } + // Blacklist the candidate to ensure it's not chosen again. candidatesChosen[s.cfg.peerPub] = struct{}{}