Limit outgoing block range request size #1061
Conversation
| private final ChainStorageClient storageClient; | ||
| private final BlockImporter blockImporter; | ||
|
|
||
| private UnsignedLong latestRequestedSlot; |
There was a problem hiding this comment.
This needs to be volatile as it's used both when the initial call to sync comes in and in response to blocks being received from the peer which may happen on different threads.
| private final BlockImporter blockImporter; | ||
|
|
||
| private UnsignedLong latestRequestedSlot; | ||
| private final CompletableFuture<PeerSyncResult> finalResult = new CompletableFuture<>(); |
There was a problem hiding this comment.
It's a bit of an anti-pattern to have a "finalResult" type future like this. Ideally we'd just chain CompletableFuture calls all the way down and just return that without having to "copy" the result to this future. The benefit of using full chaining is you guarantee never to "lose" an exception. If an exception throws anywhere in the process it will automatically be passed back up the chain. Whereas when copying the result if an exception is thrown in the whenComplete function within executeSync it will be silently ignored because the return value from requestSyncBlocks is never used.
There are times when copying to a result future is required but in this case I think we can manage it. See master...ajsutton:chain-futures We probably should talk about how we want to approach CompletableFuture handling - it can easily get unwieldy but since we're just starting to use them if we think about it a bit we can probably define some good patterns and utility methods to support them so we get it right.
There was a problem hiding this comment.
Agreed. We should talk about how we want to approach CompletableFutures.
|
|
||
| enum PeerSyncResult { | ||
| SUCCESSFUL_SYNC, | ||
| FAULTY_ADVERTISEMENT |
There was a problem hiding this comment.
We only ever use SUCCESSFUL_SYNC as the result - FAULTY_ADVERTISEMENT is returned via an exception.
PR Description
Limit outgoing block range request size