Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(blocksync): use timer instead of time.After #19

Merged
merged 1 commit into from
Mar 12, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 19 additions & 4 deletions blocksync/pool.go
Original file line number Diff line number Diff line change
Expand Up @@ -720,11 +720,11 @@ PICK_PEER_LOOP:

// Picks a second peer and sends a request to it. If the second peer is already
// set, does nothing.
func (bpr *bpRequester) pickSecondPeerAndSendRequest() {
func (bpr *bpRequester) pickSecondPeerAndSendRequest() (picked bool) {
bpr.mtx.Lock()
if bpr.secondPeerID != "" {
bpr.mtx.Unlock()
return
return false
}
peerID := bpr.peerID
bpr.mtx.Unlock()
Expand All @@ -736,7 +736,10 @@ func (bpr *bpRequester) pickSecondPeerAndSendRequest() {
bpr.mtx.Unlock()

bpr.pool.sendRequest(bpr.height, secondPeer.id)
return true
}

return false
}

// Informs the requester of a new pool's height.
Expand All @@ -761,6 +764,9 @@ OUTER_LOOP:
bpr.pickSecondPeerAndSendRequest()
}

retryTimer := time.NewTimer(requestRetrySeconds * time.Second)
defer retryTimer.Stop()

for {
select {
case <-bpr.pool.Quit():
Expand All @@ -770,7 +776,7 @@ OUTER_LOOP:
return
case <-bpr.Quit():
return
case <-time.After(requestRetrySeconds * time.Second):
case <-retryTimer.C:
if !gotBlock {
bpr.Logger.Debug("Retrying block request(s) after timeout", "height", bpr.height, "peer", bpr.peerID, "secondPeerID", bpr.secondPeerID)
bpr.reset(bpr.peerID)
Expand All @@ -787,12 +793,21 @@ OUTER_LOOP:
// If both peers returned NoBlockResponse or bad block, reschedule both
// requests. If not, wait for the other peer.
if len(bpr.requestedFrom()) == 0 {
retryTimer.Stop()
continue OUTER_LOOP
}
case newHeight := <-bpr.newHeightCh:
if !gotBlock && bpr.height-newHeight < minBlocksForSingleRequest {
// The operation is a noop if the second peer is already set. The cost is checking a mutex.
bpr.pickSecondPeerAndSendRequest()
//
// If the second peer was just set, reset the retryTimer to give the
// second peer a chance to respond.
if picked := bpr.pickSecondPeerAndSendRequest(); picked {
if !retryTimer.Stop() {
<-retryTimer.C
}
retryTimer.Reset(requestRetrySeconds * time.Second)
}
}
case <-bpr.gotBlockCh:
gotBlock = true
Expand Down
Loading