Skip to content

Commit

Permalink
Remove cancellation for Send.*AppRequest messages (#1142)
Browse files Browse the repository at this point in the history
* Remove cancellation for Send.*AppRequest messages

* Check context cancellation before sending request
  • Loading branch information
StephenButtolph authored Apr 4, 2024
1 parent 8f9dbb6 commit 004a1d2
Showing 1 changed file with 50 additions and 4 deletions.
54 changes: 50 additions & 4 deletions peer/network.go
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,12 @@ func (n *network) sendAppRequest(ctx context.Context, nodeID ids.NodeID, request
return nil
}

// If the context was cancelled, we can skip sending this request.
if err := ctx.Err(); err != nil {
n.activeAppRequests.Release(1)
return err
}

log.Debug("sending request to peer", "nodeID", nodeID, "requestLen", len(request))
n.peers.TrackPeer(nodeID)

Expand All @@ -191,8 +197,25 @@ func (n *network) sendAppRequest(ctx context.Context, nodeID ids.NodeID, request
nodeIDs.Add(nodeID)

// Send app request to [nodeID].
// On failure, release the slot from [activeAppRequests] and delete request from [outstandingRequestHandlers]
if err := n.appSender.SendAppRequest(ctx, nodeIDs, requestID, request); err != nil {
// On failure, release the slot from [activeAppRequests] and delete request
// from [outstandingRequestHandlers]
//
// Cancellation is removed from this context to avoid erroring unexpectedly.
// SendAppRequest should be non-blocking and any error other than context
// cancellation is unexpected.
//
// This guarantees that the network should never receive an unexpected
// AppResponse.
ctxWithoutCancel := context.WithoutCancel(ctx)
if err := n.appSender.SendAppRequest(ctxWithoutCancel, nodeIDs, requestID, request); err != nil {
log.Error(
"request to peer failed",
"nodeID", nodeID,
"requestID", requestID,
"requestLen", len(request),
"error", err,
)

n.activeAppRequests.Release(1)
delete(n.outstandingRequestHandlers, requestID)
return err
Expand All @@ -219,12 +242,35 @@ func (n *network) SendCrossChainRequest(ctx context.Context, chainID ids.ID, req
return nil
}

// If the context was cancelled, we can skip sending this request.
if err := ctx.Err(); err != nil {
n.activeCrossChainRequests.Release(1)
return err
}

requestID := n.nextRequestID()
n.outstandingRequestHandlers[requestID] = handler

// Send cross chain request to [chainID].
// On failure, release the slot from [activeCrossChainRequests] and delete request from [outstandingRequestHandlers].
if err := n.appSender.SendCrossChainAppRequest(ctx, chainID, requestID, request); err != nil {
// On failure, release the slot from [activeCrossChainRequests] and delete
// request from [outstandingRequestHandlers].
//
// Cancellation is removed from this context to avoid erroring unexpectedly.
// SendCrossChainAppRequest should be non-blocking and any error other than
// context cancellation is unexpected.
//
// This guarantees that the network should never receive an unexpected
// CrossChainAppResponse.
ctxWithoutCancel := context.WithoutCancel(ctx)
if err := n.appSender.SendCrossChainAppRequest(ctxWithoutCancel, chainID, requestID, request); err != nil {
log.Error(
"request to chain failed",
"chainID", chainID,
"requestID", requestID,
"requestLen", len(request),
"error", err,
)

n.activeCrossChainRequests.Release(1)
delete(n.outstandingRequestHandlers, requestID)
return err
Expand Down

0 comments on commit 004a1d2

Please sign in to comment.