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

matkt · 2020-02-24T10:52:05Z

Signed-off-by: Karim TAAM [email protected]

PR description

When Fast Sync doesn't find enough valid peers rapidly, Besu fall back to Full Sync. Besu should not do this, and instead keep searching for peers for Fast Sync "forever".

Signed-off-by: Karim TAAM <[email protected]>

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java

mbaxter · 2020-02-24T18:55:16Z

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java

-      if (retryCount > maxPivotBlockResets
-          || pivotBlockNumber.get() <= BlockHeader.GENESIS_BLOCK_NUMBER) {
+      if (confirmationTasks.size() > SUSPICIOUS_NUMBER_OF_PIVOT_BLOCK_RESETS) {
+        LOG.warn("Several attempts have been made without finding a pivot block");


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);

...

Signed-off-by: Karim TAAM <[email protected]>

matkt

@mbaxter I tried to find the best way to restart the fastsync correctly. There are two methods start and stop on the DefaultSynchronizer class . Do you think that calling this two methods during an error will restart correctly the fastsync ? I had to modify some parts of the code in order to be able to stop and restart the fastsync and I tested to throw some errors in order to check. It seems to work but I prefer to have your opinion

mbaxter · 2020-02-25T18:56:38Z

@mbaxter I tried to find the best way to restart the fastsync correctly. There are two methods start and stop on the DefaultSynchronizer class . Do you think that calling this two methods during an error will restart correctly the fastsync ? I had to modify some parts of the code in order to be able to stop and restart the fastsync and I tested to throw some errors in order to check. It seems to work but I prefer to have your opinion

Instead of reworking the top-level DefaultSynchronizer stop / start logic, which is a bit messy and changes the intention of this API, what if we rework the internal error handling in FastSyncDownloader here. We could rename this handler from handleWorldStateUnavailable to handleFailure, then we could add a conditional block that only propagates FastSyncException's, and otherwise handles other error types internally by restarting.

Signed-off-by: Karim TAAM <[email protected]>

mbaxter

LGTM - just need to update the log message to make sure its accurate

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java

Signed-off-by: Karim TAAM <[email protected]>

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java

Signed-off-by: Karim TAAM <[email protected]>

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java

Signed-off-by: Karim TAAM <[email protected]>

...th/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java

Signed-off-by: Karim TAAM <[email protected]>

…ture/BP-117-remove-fast-synf-fallback

mbaxter

Nice work! 🎉

* Plugin error stack traces (hyperledger#369) Because of how the Log4J2 api works exception stack traces were not being printed. Update to use the explicit "throwable" overloaded methods. Signed-off-by: Danno Ferrin <[email protected]> * VM Trace fixes (hyperledger#372) * correct refund addresses * correct returned memory from static precompiled calls. * update integration test * precompiles sometimes get plain old CALLs Signed-off-by: Danno Ferrin <[email protected]> * Validate private transaction before sending to enclave (hyperledger#356) Signed-off-by: Jason Frame <[email protected]> * Trace API fixes (hyperledger#377) - Correct Reporting of reverts in nested call - correct reporting and handling of value transfer in nested calls - correct handling of precompiles via DELEGATECALL & CALLCODE - Addition of precompiled contract gas costs - Re-work handling of storage writes - Initial handling of gas refunds - fix bug in DELEGATECALL tests, we don't need gas in the stack * this has a cascading effect on balances in diff tests - rework depth detection in flat trace - two new tests blocks Signed-off-by: Danno Ferrin <[email protected]> * [BOUNTY-2] Add NAT Docker Support (hyperledger#368) * add docker detection Signed-off-by: Karim TAAM <[email protected]> * add port mapping detection Signed-off-by: Karim TAAM <[email protected]> * add tests and refactor ip detection Signed-off-by: Karim TAAM <[email protected]> * clean RunnerBuilder Signed-off-by: Karim TAAM <[email protected]> * clean useless modification Signed-off-by: Karim TAAM <[email protected]> * spotless Signed-off-by: Karim TAAM <[email protected]> * resolve tests issues Signed-off-by: Karim TAAM <[email protected]> * streamline auto detection Signed-off-by: Ratan Rai Sur <[email protected]> Co-authored-by: Abdelhamid Bakhta <[email protected]> Co-authored-by: Ratan Rai Sur <[email protected]> * [PIE-1798] Priv RPC acceptance tests with stub enclave. (hyperledger#330) * [PIE-1798] Added some Privacy RPC ATs with a stub enclave. Signed-off-by: Mark Terry <[email protected]> * More specific task metrics names (hyperledger#389) A prior refactoring had accidentally removed the specific task names from the metrics labels. Signed-off-by: Danno Ferrin <[email protected]> * More trace fixes (hyperledger#386) * pop flat trace context when handling halts * Better detection of precompiled and non-executed contracts * correct from address when calling in init code * fix some exotic nesting cases * correct from field for init code calls at depth >1 * correct cost on a non-call * changelog and notes Signed-off-by: Danno Ferrin <[email protected]> * adding the plugin-api javadoc jar at the root level (hyperledger#378) Signed-off-by: Joshua Fernandes <[email protected]> * BESU-56: remove erroneous links to errorprone checks (hyperledger#385) Signed-off-by: Antoine Toulme <[email protected]> * Changelog entry for multi-tenancy feature (hyperledger#394) Signed-off-by: Edward Evans <[email protected]> Signed-off-by: Jason Frame <[email protected]> * 1.5 RC Changelog (hyperledger#395) * 1.5 RC changelog additions Signed-off-by: Sally MacFarlane <[email protected]> * Fixed link (hyperledger#393) Signed-off-by: Madeline <[email protected]> Co-authored-by: Edward <[email protected]> * docker changelog (hyperledger#391) * docker changelog Signed-off-by: Ratan Rai Sur <[email protected]> * address comments Signed-off-by: Ratan Rai Sur <[email protected]> Co-authored-by: MadelineMurray <[email protected]> * Added known bug to changelog (hyperledger#388) * Added known bug Signed-off-by: Madeline <[email protected]> * Added another known bug Signed-off-by: Madeline <[email protected]> * updating version to 1.4.0-rc1 (hyperledger#397) Signed-off-by: Joshua Fernandes <[email protected]> * updating verion to 1.4.1-snapshot (hyperledger#398) Signed-off-by: Joshua Fernandes <[email protected]> * [BESU-169] cache logs bloom filters automatically. (hyperledger#367) * First iteration. Draft PR. Signed-off-by: Abdelhamid Bakhta <[email protected]> * fix SPDX header Signed-off-by: Abdelhamid Bakhta <[email protected]> * Use block broadcaster to index log bloom. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Remove useless toString method Signed-off-by: Abdelhamid Bakhta <[email protected]> * spotless apply Signed-off-by: Abdelhamid Bakhta <[email protected]> * cacheLogsBloomForBlockHeader Signed-off-by: Abdelhamid Bakhta <[email protected]> * spotless apply Signed-off-by: Abdelhamid Bakhta <[email protected]> * ensurePreviousSegmentsArePresent Signed-off-by: Abdelhamid Bakhta <[email protected]> * Added CLI flag to enable / disable automatic logs bloom indexing. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Create cache directory and cache file if not exist. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Fix acceptance test Signed-off-by: Abdelhamid Bakhta <[email protected]> * Write cache for block only if block is new canonical head. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Handling of chain reorg. Signed-off-by: Abdelhamid Bakhta <[email protected]> * fix Signed-off-by: Abdelhamid Bakhta <[email protected]> * sportless apply Signed-off-by: Abdelhamid Bakhta <[email protected]> * Address PR comments. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Remove unused constant. Signed-off-by: Abdelhamid Bakhta <[email protected]> * spotless apply Signed-off-by: Abdelhamid Bakhta <[email protected]> * [BESU-25] Use Devp2p Ping packets at v5 (hyperledger#392) Broadcast that we support snappy compression Signed-off-by: Danno Ferrin <[email protected]> * Rename logs bloom indexer to log bloom cache to match CLI flag. (hyperledger#401) Changes class names, variables, and CLI flags as needed. Signed-off-by: Danno Ferrin <[email protected]> * [PIE-1798] Fail cases for multitenancy ATs (hyperledger#400) Signed-off-by: Mark Terry <[email protected]> * Private state update metadata and migration (hyperledger#404) (backport from release-1.4) Private state update metadata and migration Signed-off-by: Lucas Saldanha <[email protected]> * [PIE-2303] Automatic log bloom caching - Remove usage of pending file. (hyperledger#407) * Don't use pending file. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Don't use pending file. Signed-off-by: Abdelhamid Bakhta <[email protected]> * Create a custom error when plugin is not found. (hyperledger#409) Signed-off-by: Abdelhamid Bakhta <[email protected]> * Rename method (hyperledger#412) * rename the method isPersistingState to isPersistingPrivateState because that is what it is used for Signed-off-by: Stefan Pingel <[email protected]> * rename the method isPersistingState to isPersistingPrivateState because that is what it is used for Signed-off-by: Stefan Pingel <[email protected]> * rename the method isPersistingState to isPersistingPrivateState because that is what it is used for Signed-off-by: Stefan Pingel <[email protected]> * [BOUNTY-4] Add NAT Kubernetes Support (hyperledger#410) * add kubernetes support Signed-off-by: Karim TAAM <[email protected]> * fix review issues Signed-off-by: Karim TAAM <[email protected]> * LogBloomCache - make sure the current segment is filled (hyperledger#411) Make sure we cache the current cache segment with all of the data from the beginning of the segment. Use a flip file approach since it will be a partial file until done. Signed-off-by: Danno Ferrin <[email protected]> Co-authored-by: Abdelhamid Bakhta <[email protected]> * Reduce recaching in Transaction Log Bloom Filter Cache (hyperledger#415) Do a cursory cache check at start up (file is present and correct size) instead of re-generating the cache at startup. Signed-off-by: Danno Ferrin <[email protected]> * fix order of nat detector (hyperledger#414) Signed-off-by: Karim TAAM <[email protected]> * Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator (#23) (hyperledger#382) * Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator Change SLOAD_GAS cost in Aztlan Gas Calculator from 800 to 200 and update functions that use SLOAD_GAS. Signed-off-by: edwardmack <[email protected]> * Update SLOAD_GAS cost to 200 in Aztlan Gas Calculator Change SLOAD_GAS cost in Aztlan Gas Calculator from 800 to 200 and update functions that use SLOAD_GAS. Signed-off-by: edwardmack <[email protected]> Signed-off-by: Edward Mack <[email protected]> * remove overrides removed overrides of calculateStorageCost and calculateStorageRefundAmount in AztlanGasCalculator because these were causing aztlan fork not to sync with kotti testnet. Signed-off-by: Edward Mack <[email protected]> * merge Signed-off-by: Edward Mack <[email protected]> * Remove unused ExecutorService init/termination (hyperledger#419) Signed-off-by: Horacio Mijail Anton Quiles <[email protected]> Co-authored-by: CJ Hare <[email protected]> * Implement Eth/64 (hyperledger#425) Wire in the fork identifier into the status messages as Eth64. Signed-off-by: Danno Ferrin <[email protected]> * Adds priv_getcode (hyperledger#428) * Adds priv_getcode Signed-off-by: Joshua Richardson <[email protected]> * BESU-146 - check if success and return errorResponse otherwise (hyperledger#424) Signed-off-by: Anthony Buckle <[email protected]> Co-authored-by: CJ Hare <[email protected]> * fixed typos (hyperledger#429) Signed-off-by: Sally MacFarlane <[email protected]> Co-authored-by: CJ Hare <[email protected]> * roll back on ALL-CAPS.md files for TSC proposal (hyperledger#376) Signed-off-by: Felipe Faraggi <[email protected]> * updating the changelog with 1.4.0 details (hyperledger#431) Signed-off-by: Joshua Fernandes <[email protected]> * [BESU-194] Remove max pivot block resets during fast sync (hyperledger#427) * remove max pivot block resets during fast sync * increase max retry number and fix test * change logs in the handleFailure method * change logs related to suspicious number of retries Signed-off-by: Karim TAAM <[email protected]> Co-authored-by: Danno Ferrin <[email protected]> Co-authored-by: Jason Frame <[email protected]> Co-authored-by: Karim T. <[email protected]> Co-authored-by: Abdelhamid Bakhta <[email protected]> Co-authored-by: Ratan Rai Sur <[email protected]> Co-authored-by: mark-terry <[email protected]> Co-authored-by: Joshua Fernandes <[email protected]> Co-authored-by: Antoine Toulme <[email protected]> Co-authored-by: Sally MacFarlane <[email protected]> Co-authored-by: MadelineMurray <[email protected]> Co-authored-by: Edward <[email protected]> Co-authored-by: Lucas Saldanha <[email protected]> Co-authored-by: pinges <[email protected]> Co-authored-by: Horacio Mijail Antón Quiles <[email protected]> Co-authored-by: CJ Hare <[email protected]> Co-authored-by: anthonybuckle <[email protected]> Co-authored-by: Felipe Faraggi <[email protected]>

* Rebase eip6110 to Cancun * Rename DepositV1 to DepositReceiptV1 * Fix broken link

remove max pivot block resets during fast sync

6a964f9

Signed-off-by: Karim TAAM <[email protected]>

matkt marked this pull request as ready for review February 24, 2020 11:27

shemnon reviewed Feb 24, 2020

View reviewed changes

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java Show resolved Hide resolved

mbaxter reviewed Feb 24, 2020

View reviewed changes

MadelineMurray mentioned this pull request Feb 24, 2020

Doc behaviour when fast-sync fails hyperledger/besu-docs#203

Closed

increase retry number and add retry for fast sync

1fe8583

Signed-off-by: Karim TAAM <[email protected]>

matkt commented Feb 25, 2020

View reviewed changes

matkt added 2 commits February 26, 2020 12:22

fix review issues

b2a4326

Signed-off-by: Karim TAAM <[email protected]>

increase max retry number and fix test

66e56b3

Signed-off-by: Karim TAAM <[email protected]>

mbaxter reviewed Feb 26, 2020

View reviewed changes

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java Outdated Show resolved Hide resolved

...m/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/PivotBlockRetriever.java Outdated Show resolved Hide resolved

matkt added 2 commits February 26, 2020 16:03

change logs in the handleFailure method

0e68430

Signed-off-by: Karim TAAM <[email protected]>

change logs related to suspicious number of retries

1ce4949

Signed-off-by: Karim TAAM <[email protected]>

mbaxter approved these changes Feb 26, 2020

View reviewed changes

matkt added 2 commits February 26, 2020 20:06

add a delay before each fast sync retry

e74d3d4

Signed-off-by: Karim TAAM <[email protected]>

add a timeout for fast sync retry

e296443

Signed-off-by: Karim TAAM <[email protected]>

mbaxter reviewed Feb 26, 2020

View reviewed changes

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java Outdated Show resolved Hide resolved

change timeout for fast sync retry

8438dac

Signed-off-by: Karim TAAM <[email protected]>

mbaxter reviewed Feb 26, 2020

View reviewed changes

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java Outdated Show resolved Hide resolved

...um/eth/src/main/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloader.java Outdated Show resolved Hide resolved

fix log add error

ecbe0ca

Signed-off-by: Karim TAAM <[email protected]>

matkt force-pushed the feature/BP-117-remove-fast-synf-fallback branch from f42b755 to ecbe0ca Compare February 26, 2020 21:17

matkt added 2 commits February 26, 2020 22:29

fix review issues

7f0d07c

Signed-off-by: Karim TAAM <[email protected]>

resolve pipeline

cbd012a

Signed-off-by: Karim TAAM <[email protected]>

mbaxter reviewed Feb 26, 2020

View reviewed changes

...th/src/test/java/org/hyperledger/besu/ethereum/eth/sync/fastsync/FastSyncDownloaderTest.java Outdated Show resolved Hide resolved

matkt added 2 commits February 27, 2020 10:32

update test for restart fastsync mechanism

e07dc40

Signed-off-by: Karim TAAM <[email protected]>

Merge branch 'master' of https://github.com/hyperledger/besu into fea…

cf89606

…ture/BP-117-remove-fast-synf-fallback

mbaxter approved these changes Feb 27, 2020

View reviewed changes

AbdelStark merged commit 8ced45c into hyperledger:master Feb 27, 2020

matkt deleted the feature/BP-117-remove-fast-synf-fallback branch March 9, 2020 09:20

siladu pushed a commit to siladu/besu that referenced this pull request Oct 28, 2024

Rebase eip6110 to Cancun (hyperledger#427)

4da6a02

* Rebase eip6110 to Cancun * Rename DepositV1 to DepositReceiptV1 * Fix broken link

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

matkt commented Feb 24, 2020

mbaxter Feb 24, 2020

matkt left a comment

mbaxter commented Feb 25, 2020

mbaxter left a comment

mbaxter left a comment

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

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

Conversation

matkt commented Feb 24, 2020

PR description

mbaxter Feb 24, 2020

Choose a reason for hiding this comment

matkt left a comment

Choose a reason for hiding this comment

mbaxter commented Feb 25, 2020

mbaxter left a comment

Choose a reason for hiding this comment

mbaxter left a comment

Choose a reason for hiding this comment