From 9249c9aa2eaa0f60642f00fe0283668bbf0be3de Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Wed, 6 Feb 2019 06:18:50 +1000 Subject: [PATCH] [NC-2180] Wait for a peer with an estimated chain height before selecting a pivot block (#772) Signed-off-by: Adrian Sutton --- .../eth/sync/fastsync/FastSyncActions.java | 23 ++++++++++++++----- .../eth/sync/fastsync/FastSyncDownloader.java | 2 +- .../sync/fastsync/FastSyncActionsTest.java | 21 +++++++++-------- .../sync/fastsync/FastSyncDownloaderTest.java | 10 ++++---- 4 files changed, 34 insertions(+), 22 deletions(-) diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java index 29ed758362..5505a5cfea 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActions.java @@ -12,8 +12,8 @@ */ package tech.pegasys.pantheon.ethereum.eth.sync.fastsync; +import static java.util.concurrent.CompletableFuture.completedFuture; import static tech.pegasys.pantheon.ethereum.eth.sync.fastsync.FastSyncError.CHAIN_TOO_SHORT; -import static tech.pegasys.pantheon.ethereum.eth.sync.fastsync.FastSyncError.NO_PEERS_AVAILABLE; import tech.pegasys.pantheon.ethereum.ProtocolContext; import tech.pegasys.pantheon.ethereum.core.BlockHeader; @@ -27,6 +27,7 @@ import tech.pegasys.pantheon.metrics.LabelledMetric; import tech.pegasys.pantheon.metrics.OperationTimer; import tech.pegasys.pantheon.util.ExceptionUtils; +import java.time.Duration; import java.util.OptionalLong; import java.util.concurrent.CompletableFuture; import java.util.concurrent.TimeoutException; @@ -76,6 +77,8 @@ public class FastSyncActions { "Fast sync timed out before minimum peer count was reached. Continuing with reduced peers."); result.complete(null); } else { + LOG.warn( + "Maximum wait time for fast sync reached but no peers available. Continuing to wait for any available peer."); waitForAnyPeer() .thenAccept(result::complete) .exceptionally( @@ -97,8 +100,6 @@ public class FastSyncActions { } private CompletableFuture waitForAnyPeer() { - LOG.warn( - "Maximum wait time for fast sync reached but no peers available. Continuing to wait for any available peer."); final CompletableFuture result = new CompletableFuture<>(); waitForAnyPeer(result); return result; @@ -120,10 +121,11 @@ public class FastSyncActions { }); } - public FastSyncState selectPivotBlock() { + public CompletableFuture selectPivotBlock() { return ethContext .getEthPeers() .bestPeer() + .filter(peer -> peer.chainState().hasEstimatedHeight()) .map( peer -> { final long pivotBlockNumber = @@ -132,10 +134,19 @@ public class FastSyncActions { throw new FastSyncException(CHAIN_TOO_SHORT); } else { LOG.info("Selecting block number {} as fast sync pivot block.", pivotBlockNumber); - return new FastSyncState(OptionalLong.of(pivotBlockNumber)); + return completedFuture(new FastSyncState(OptionalLong.of(pivotBlockNumber))); } }) - .orElseThrow(() -> new FastSyncException(NO_PEERS_AVAILABLE)); + .orElseGet(this::retrySelectPivotBlockAfterDelay); + } + + private CompletableFuture retrySelectPivotBlockAfterDelay() { + LOG.info("Waiting for peer with known chain height"); + return ethContext + .getScheduler() + .scheduleFutureTask( + () -> waitForAnyPeer().thenCompose(ignore -> selectPivotBlock()), + Duration.ofSeconds(1)); } public CompletableFuture downloadPivotBlockHeader( diff --git a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloader.java b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloader.java index 73502ceb23..3424c96656 100644 --- a/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloader.java +++ b/ethereum/eth/src/main/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloader.java @@ -34,7 +34,7 @@ public class FastSyncDownloader { LOG.info("Fast sync enabled"); return fastSyncActions .waitForSuitablePeers() - .thenApply(state -> fastSyncActions.selectPivotBlock()) + .thenCompose(state -> fastSyncActions.selectPivotBlock()) .thenCompose(fastSyncActions::downloadPivotBlockHeader) .thenCompose(this::downloadChainAndWorldState); } diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java index 3ba1eacd10..b5df42db1b 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncActionsTest.java @@ -15,7 +15,6 @@ package tech.pegasys.pantheon.ethereum.eth.sync.fastsync; import static org.assertj.core.api.Assertions.assertThat; import static org.assertj.core.api.Assertions.assertThatThrownBy; import static tech.pegasys.pantheon.ethereum.eth.sync.fastsync.FastSyncError.CHAIN_TOO_SHORT; -import static tech.pegasys.pantheon.ethereum.eth.sync.fastsync.FastSyncError.NO_PEERS_AVAILABLE; import static tech.pegasys.pantheon.metrics.noop.NoOpMetricsSystem.NO_OP_LABELLED_TIMER; import tech.pegasys.pantheon.ethereum.ProtocolContext; @@ -49,9 +48,6 @@ public class FastSyncActionsTest { .fastSyncPivotDistance(1000) .build(); - private ProtocolSchedule protocolSchedule; - private ProtocolContext protocolContext; - private final LabelledMetric ethTasksTimer = NO_OP_LABELLED_TIMER; private final AtomicInteger timeoutCount = new AtomicInteger(0); private FastSyncActions fastSyncActions; @@ -63,8 +59,8 @@ public class FastSyncActionsTest { final BlockchainSetupUtil blockchainSetupUtil = BlockchainSetupUtil.forTesting(); blockchainSetupUtil.importAllBlocks(); blockchain = blockchainSetupUtil.getBlockchain(); - protocolSchedule = blockchainSetupUtil.getProtocolSchedule(); - protocolContext = blockchainSetupUtil.getProtocolContext(); + final ProtocolSchedule protocolSchedule = blockchainSetupUtil.getProtocolSchedule(); + final ProtocolContext protocolContext = blockchainSetupUtil.getProtocolContext(); ethProtocolManager = EthProtocolManagerTestUtil.create( blockchain, @@ -111,14 +107,19 @@ public class FastSyncActionsTest { public void selectPivotBlockShouldSelectBlockPivotDistanceFromBestPeer() { EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 5000); - final FastSyncState result = fastSyncActions.selectPivotBlock(); + final CompletableFuture result = fastSyncActions.selectPivotBlock(); final FastSyncState expected = new FastSyncState(OptionalLong.of(4000)); - assertThat(result).isEqualTo(expected); + assertThat(result).isCompletedWithValue(expected); } @Test - public void selectPivotBlockShouldFailIfNoPeersAreAvailable() { - assertThrowsFastSyncException(NO_PEERS_AVAILABLE, fastSyncActions::selectPivotBlock); + public void selectPivotBlockShouldWaitAndRetryIfNoPeersAreAvailable() { + final CompletableFuture result = fastSyncActions.selectPivotBlock(); + assertThat(result).isNotDone(); + + EthProtocolManagerTestUtil.createPeer(ethProtocolManager, 5000); + final FastSyncState expected = new FastSyncState(OptionalLong.of(4000)); + assertThat(result).isCompletedWithValue(expected); } @Test diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java index cfc9fc7c3d..33a9dcf751 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java @@ -51,7 +51,7 @@ public class FastSyncDownloaderTest { final FastSyncState downloadPivotBlockHeaderState = new FastSyncState(OptionalLong.of(50), Optional.of(pivotBlockHeader)); when(fastSyncActions.waitForSuitablePeers()).thenReturn(COMPLETE); - when(fastSyncActions.selectPivotBlock()).thenReturn(selectPivotBlockState); + when(fastSyncActions.selectPivotBlock()).thenReturn(completedFuture(selectPivotBlockState)); when(fastSyncActions.downloadPivotBlockHeader(selectPivotBlockState)) .thenReturn(completedFuture(downloadPivotBlockHeaderState)); when(fastSyncActions.downloadChain(downloadPivotBlockHeaderState)).thenReturn(COMPLETE); @@ -105,7 +105,7 @@ public class FastSyncDownloaderTest { final FastSyncState downloadPivotBlockHeaderState = new FastSyncState(OptionalLong.of(50), Optional.of(pivotBlockHeader)); when(fastSyncActions.waitForSuitablePeers()).thenReturn(COMPLETE); - when(fastSyncActions.selectPivotBlock()).thenReturn(selectPivotBlockState); + when(fastSyncActions.selectPivotBlock()).thenReturn(completedFuture(selectPivotBlockState)); when(fastSyncActions.downloadPivotBlockHeader(selectPivotBlockState)) .thenReturn(completedFuture(downloadPivotBlockHeaderState)); when(fastSyncActions.downloadChain(downloadPivotBlockHeaderState)).thenReturn(chainFuture); @@ -137,7 +137,7 @@ public class FastSyncDownloaderTest { final FastSyncState downloadPivotBlockHeaderState = new FastSyncState(OptionalLong.of(50), Optional.of(pivotBlockHeader)); when(fastSyncActions.waitForSuitablePeers()).thenReturn(COMPLETE); - when(fastSyncActions.selectPivotBlock()).thenReturn(selectPivotBlockState); + when(fastSyncActions.selectPivotBlock()).thenReturn(completedFuture(selectPivotBlockState)); when(fastSyncActions.downloadPivotBlockHeader(selectPivotBlockState)) .thenReturn(completedFuture(downloadPivotBlockHeaderState)); when(fastSyncActions.downloadChain(downloadPivotBlockHeaderState)).thenReturn(chainFuture); @@ -169,7 +169,7 @@ public class FastSyncDownloaderTest { final FastSyncState downloadPivotBlockHeaderState = new FastSyncState(OptionalLong.of(50), Optional.of(pivotBlockHeader)); when(fastSyncActions.waitForSuitablePeers()).thenReturn(COMPLETE); - when(fastSyncActions.selectPivotBlock()).thenReturn(selectPivotBlockState); + when(fastSyncActions.selectPivotBlock()).thenReturn(completedFuture(selectPivotBlockState)); when(fastSyncActions.downloadPivotBlockHeader(selectPivotBlockState)) .thenReturn(completedFuture(downloadPivotBlockHeaderState)); when(fastSyncActions.downloadChain(downloadPivotBlockHeaderState)).thenReturn(chainFuture); @@ -200,7 +200,7 @@ public class FastSyncDownloaderTest { final FastSyncState downloadPivotBlockHeaderState = new FastSyncState(OptionalLong.of(50), Optional.of(pivotBlockHeader)); when(fastSyncActions.waitForSuitablePeers()).thenReturn(COMPLETE); - when(fastSyncActions.selectPivotBlock()).thenReturn(selectPivotBlockState); + when(fastSyncActions.selectPivotBlock()).thenReturn(completedFuture(selectPivotBlockState)); when(fastSyncActions.downloadPivotBlockHeader(selectPivotBlockState)) .thenReturn(completedFuture(downloadPivotBlockHeaderState)); when(fastSyncActions.downloadChain(downloadPivotBlockHeaderState)).thenReturn(chainFuture);