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

Attempt Pending Request to BUSY && ASSIGNED && DISCONNECTED Peer #1321

Merged
merged 4 commits into from
Aug 20, 2020

Conversation

RatanRSur
Copy link
Contributor

Any process that depends on pre-assigned peer tasks is liable to grow our pendingRequests out of control.

When we attempt to execute a pending transaction, we check for that peers availability:

final Optional<EthPeer> selectedPeer =
leastBusySuitablePeer.filter(EthPeer::hasAvailableRequestCapacity);
selectedPeer.ifPresent(this::sendRequest);
return selectedPeer.isPresent();
}
}

However, if that peer was disconnected while there were max outstanding requsts, we'll never reattempt it as selectedPeer will be empty. Furthermore, we'll return false and keep that pending request in our list of requests.

With this change we indicate that we aren't waiting for any outstanding requests when we disconnect from a peer, allowing the existing logic that triggers when peers are disconnected to be hit.

The getter I added just for testing purposes was to get around the inadequecies of the MockPeerConnection. There's a whole lot of callback plumbing that's needed for a simple ethPeer.disconnect to do the same thing it does when we call it in our production code.

Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
Signed-off-by: Ratan Rai Sur <ratan.r.sur@gmail.com>
@RatanRSur RatanRSur merged commit 3a39fbb into hyperledger:master Aug 20, 2020
@RatanRSur RatanRSur deleted the abort-assigned-peer-request branch August 20, 2020 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants