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.

Sign up for GitHub

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

[BESU-194] Remove max pivot block resets during fast sync #427

Merged
merged 14 commits into from
Feb 27, 2020
Merged

[BESU-194] Remove max pivot block resets during fast sync #427

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
6a964f9
remove max pivot block resets during fast sync
matkt Feb 24, 2020
1fe8583
increase retry number and add retry for fast sync
matkt Feb 25, 2020
b2a4326
fix review issues
matkt Feb 26, 2020
66e56b3
increase max retry number and fix test
matkt Feb 26, 2020
0e68430
change logs in the handleFailure method
matkt Feb 26, 2020
1ce4949
change logs related to suspicious number of retries
matkt Feb 26, 2020
e74d3d4
add a delay before each fast sync retry
matkt Feb 26, 2020
e296443
add a timeout for fast sync retry
matkt Feb 26, 2020
8438dac
change timeout for fast sync retry
matkt Feb 26, 2020
ecbe0ca
fix log add error
matkt Feb 26, 2020
7f0d07c
fix review issues
matkt Feb 26, 2020
cbd012a
resolve pipeline
matkt Feb 26, 2020
e07dc40
update test for restart fastsync mechanism
matkt Feb 27, 2020
cf89606
Merge branch 'master' of https://github.com/hyperledger/besu into fea…
matkt Feb 27, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
19 changes: 15 additions & 4 deletions ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,11 @@ private TrailingPeerRequirements calculateTrailingPeerRequirements() {
public void start() {
if (running.compareAndSet(false, true)) {
LOG.info("Starting synchronizer.");
blockPropagationManager.start();
try {
blockPropagationManager.start();
} catch (IllegalStateException e) {
LOG.info("Attempt to start an already started block propagation manager");
}
if (fastSyncDownloader.isPresent()) {
fastSyncDownloader.get().start().whenComplete(this::handleFastSyncResult);
} else {
Expand Down Expand Up @@ -162,16 +166,23 @@ private void handleFastSyncResult(final FastSyncState result, final Throwable er
LOG.error(
"Fast sync failed ({}), switching to full sync.",
((FastSyncException) rootCause).getError());
fastSyncDownloader.ifPresent(FastSyncDownloader::deleteFastSyncState);
startFullSync();
} else if (error != null) {
LOG.error("Fast sync failed, switching to full sync.", error);
LOG.error("Fast sync failed, trying to restart.", error);
restartFastSync();
} else {
LOG.info(
"Fast sync completed successfully with pivot block {}",
result.getPivotBlockNumber().getAsLong());
fastSyncDownloader.ifPresent(FastSyncDownloader::deleteFastSyncState);
startFullSync();
}
fastSyncDownloader.ifPresent(FastSyncDownloader::deleteFastSyncState);
}

startFullSync();
private void restartFastSync() {
stop();
start();
}

private void startFullSync() {
Expand Down
6 changes: 6 additions & 0 deletions ...th/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@ public class PivotBlockRetriever<C> {
private static final Logger LOG = LogManager.getLogger();
public static final int MAX_QUERY_RETRIES_PER_PEER = 3;
private static final int DEFAULT_MAX_PIVOT_BLOCK_RESETS = 5;
private static final int SUSPICIOUS_NUMBER_OF_RETRIES = 5;

private final EthContext ethContext;
private final MetricsSystem metricsSystem;
Expand Down Expand Up @@ -148,6 +149,11 @@ private void handleContestedPivotBlock(final long contestedBlockNumber) {
LOG.info("Received conflicting pivot blocks for {}.", contestedBlockNumber);

final int retryCount = confirmationTasks.size();

if ((retryCount % SUSPICIOUS_NUMBER_OF_RETRIES) == 0) {
LOG.warn("Several attempts have been made without finding a pivot block");
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd print a warning periodically (every SUSPICIOUS_NUMBER_OF_PIVOT_BLOCK_RESETS retries?) rather than on every individual retry. Also, confirmationTasks can now grow very large, so we'll probably need to rework how we track tasks so we don't hold onto every confirmation task we created.

Alternatively, I wonder if we could alleviate the problem of prematurely switching to full sync by just increasing DEFAULT_MAX_PIVOT_BLOCK_RESETS and making sure we retry fast sync here:

besu/ethereum/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/DefaultSynchronizer.java

Line 166 in 7fe1d47

LOG.error("Fast sync failed, switching to full sync.", error);
...

matkt marked this conversation as resolved.
Show resolved Hide resolved
}

if (retryCount > maxPivotBlockResets
|| pivotBlockNumber.get() <= BlockHeader.GENESIS_BLOCK_NUMBER) {
LOG.info("Max retries reached, cancel pivot block download.");
matkt marked this conversation as resolved.
Show resolved Hide resolved
Expand Down