From d04b097882f6040bad906a95aca616d9638497f4 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Mon, 11 Feb 2019 15:13:16 +1000 Subject: [PATCH 1/2] Attempt to fix intermittency in FullSyncDownloaderTest. --- .../sync/fullsync/FullSyncDownloaderTest.java | 77 +++++++------------ 1 file changed, 26 insertions(+), 51 deletions(-) diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index 1562ad9455..1af37f933d 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java @@ -125,15 +125,12 @@ public void syncsToBetterChain_multipleSegments() { final FullSyncDownloader downloader = downloader(syncConfig); downloader.start(); - while (!syncState.syncTarget().isPresent()) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); - while (localBlockchain.getChainHeadBlockNumber() < targetBlock) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); } @@ -157,15 +154,12 @@ public void syncsToBetterChain_singleSegment() { final FullSyncDownloader downloader = downloader(syncConfig); downloader.start(); - while (!syncState.syncTarget().isPresent()) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); - while (localBlockchain.getChainHeadBlockNumber() < targetBlock) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); } @@ -189,15 +183,12 @@ public void syncsToBetterChain_singleSegmentOnBoundary() { final FullSyncDownloader downloader = downloader(syncConfig); downloader.start(); - while (!syncState.syncTarget().isPresent()) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); - while (localBlockchain.getChainHeadBlockNumber() < targetBlock) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(targetBlock); } @@ -219,9 +210,7 @@ public void doesNotSyncToWorseChain() { peer.respond(responder); assertThat(syncState.syncTarget()).isNotPresent(); - while (peer.hasOutstandingRequests()) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork(responder, peer::hasOutstandingRequests); assertThat(syncState.syncTarget()).isNotPresent(); verify(localBlockchain, times(0)).appendBlock(any(), any()); @@ -256,9 +245,8 @@ public void syncsToBetterChain_fromFork() { final FullSyncDownloader downloader = downloader(syncConfig); downloader.start(); - while (localBlockchain.getChainHeadBlockNumber() < targetBlock) { - peer.respond(responder); - } + peer.respondWhileOtherThreadsWork( + responder, () -> localBlockchain.getChainHeadBlockNumber() < targetBlock); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peer.getEthPeer()); @@ -330,9 +318,7 @@ public void switchesSyncTarget_betterHeight() { downloader.start(); // Process until the sync target is selected - while (!syncState.syncTarget().isPresent()) { - peerA.respond(responder); - } + peerA.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peerA.getEthPeer()); @@ -370,9 +356,7 @@ public void doesNotSwitchSyncTarget_betterHeightUnderThreshold() { downloader.start(); // Process until the sync target is selected - while (!syncState.syncTarget().isPresent()) { - bestPeer.respond(responder); - } + bestPeer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(bestPeer.getEthPeer()); @@ -410,9 +394,7 @@ public void switchesSyncTarget_betterTd() { downloader.start(); // Process until the sync target is selected - while (!syncState.syncTarget().isPresent()) { - peerA.respond(responder); - } + peerA.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(peerA.getEthPeer()); @@ -457,9 +439,7 @@ public void doesNotSwitchSyncTarget_betterTdUnderThreshold() { downloader.start(); // Process until the sync target is selected - while (!syncState.syncTarget().isPresent()) { - bestPeer.respond(responder); - } + bestPeer.respondWhileOtherThreadsWork(responder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(bestPeer.getEthPeer()); @@ -512,13 +492,7 @@ public void recoversFromSyncTargetDisconnect() { downloader.start(); // Process through sync target selection - await() - .atMost(10, TimeUnit.SECONDS) - .untilAsserted( - () -> { - bestPeer.respond(bestResponder); - assertThat(syncState.syncTarget()).isNotEmpty(); - }); + bestPeer.respondWhileOtherThreadsWork(bestResponder, () -> !syncState.syncTarget().isPresent()); assertThat(syncState.syncTarget()).isPresent(); assertThat(syncState.syncTarget().get().peer()).isEqualTo(bestPeer.getEthPeer()); @@ -534,6 +508,7 @@ public void recoversFromSyncTargetDisconnect() { // Process through the first import await() .atMost(10, TimeUnit.SECONDS) + .pollInterval(100, TimeUnit.MILLISECONDS) .untilAsserted( () -> { if (!bestPeer.respond(bestResponder)) { @@ -543,20 +518,20 @@ public void recoversFromSyncTargetDisconnect() { .isNotEqualTo(localChainHeadAtStart); }); + // Sanity check that we haven't already passed the second best peer + assertThat(localBlockchain.getChainHeadBlockNumber()).isLessThan(secondBestPeerChainHead); + // Disconnect peer ethProtocolManager.handleDisconnect( bestPeer.getPeerConnection(), DisconnectReason.TOO_MANY_PEERS, true); // Downloader should recover and sync to next best peer, but it may stall // for 10 seconds first (by design). - await() - .atMost(30, TimeUnit.SECONDS) - .untilAsserted( - () -> { - secondBestPeer.respond(secondBestResponder); - assertThat(localBlockchain.getChainHeadBlockNumber()) - .isEqualTo(secondBestPeerChainHead); - }); + secondBestPeer.respondWhileOtherThreadsWork( + secondBestResponder, + () -> localBlockchain.getChainHeadBlockNumber() != secondBestPeerChainHead); + + assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(secondBestPeerChainHead); } @Test From ec586bd3819bcfebb00894f40a52ed7f10de4dd7 Mon Sep 17 00:00:00 2001 From: Adrian Sutton Date: Tue, 12 Feb 2019 06:06:11 +1000 Subject: [PATCH 2/2] Fix spotless. --- .../ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java index 1af37f933d..982141008a 100644 --- a/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java +++ b/ethereum/eth/src/test/java/tech/pegasys/pantheon/ethereum/eth/sync/fullsync/FullSyncDownloaderTest.java @@ -530,7 +530,7 @@ public void recoversFromSyncTargetDisconnect() { secondBestPeer.respondWhileOtherThreadsWork( secondBestResponder, () -> localBlockchain.getChainHeadBlockNumber() != secondBestPeerChainHead); - + assertThat(localBlockchain.getChainHeadBlockNumber()).isEqualTo(secondBestPeerChainHead); }