Add chain data pruning experimental feature #4686

wcgcyx · 2022-11-16T04:21:20Z

PR description

This PR introduces a new feature that allows besu to prune old chain data. Three experimental CLI options are introduced as well: --Xchain-pruning-enabled to turn on/off pruning, --Xchain-pruning-blocks-retained to specify the number of recent blocks to retain and --Xchain-pruning-frequency to specify how often to perform chain data pruning.

Fixed Issue(s)

Linked to #4476

Documentation

I thought about documentation and added the doc-change-required label to this PR if
updates are required.

Changelog

I thought about the changelog and included a changelog update if required.

Signed-off-by: wcgcyx <[email protected]>

Signed-off-by: Zhenyang Shi <[email protected]>

Signed-off-by: wcgcyx <[email protected]>

Signed-off-by: Zhenyang Shi <[email protected]>

CHANGELOG.md

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/ChainDataPruningOptions.java

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

.../src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueSegmentIdentifier.java

Signed-off-by: wcgcyx <[email protected]>

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

jframe · 2022-11-23T06:34:11Z

...g/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineForkchoiceUpdated.java

@@ -116,7 +117,8 @@ public JsonRpcResponse syncResponse(final JsonRpcRequestContext requestContext)
    }

    // TODO: post-merge cleanup, this should be unnecessary after merge
-    if (!mergeCoordinator.latestValidAncestorDescendsFromTerminal(newHead.get())) {
+    if (!mergeCoordinator.latestValidAncestorDescendsFromTerminal(newHead.get())
+        && !ChainDataPruner.isPruningEnabled()) {


Would be nicer to drive this based on the chain data pruning enabled flag rather than setting and using a static global field

Yea, I tried that a bit but it seems like that would involve bringing besu command into the protocol context. I consider it to be a temporary solution until #4703 is merged. Then, we could just do a cleanup to get rid of this global field completely.

We shouldn't need to bring the entire Besu command into the protocol context. At most, the chain pruning config values would need to be passed through. A compromise could be to make the ChainPruner a singleton instead.

Actually, It seems cleaner to add a flag in merge context instead, following what @gfukushima did in #4735

updated to have a temporary flag in protocol context instead. Global variable gets removed.

...java/org/hyperledger/besu/ethereum/api/jsonrpc/internal/methods/engine/EngineNewPayload.java

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

.../src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueSegmentIdentifier.java

Signed-off-by: wcgcyx <[email protected]>

Signed-off-by: Zhenyang Shi <[email protected]>

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/ChainDataPruningOptions.java

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

Signed-off-by: wcgcyx <[email protected]>

Signed-off-by: Zhenyang Shi <[email protected]>

jframe · 2022-12-05T06:40:20Z

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

+        .isChainDataPruningEnabled(unstableChainDataPruningOptions.getChainDataPruningEnabled())
+        .chainDataPruningBlocksRetained(
+            unstableChainDataPruningOptions.getChainDataPruningBlocksRetained())
+        .chainDataPruningFrequency(
+            unstableChainDataPruningOptions.getChainDataPruningBlocksFrequency());


Can domain object be created instead so that 3 values don't need to passed through

jframe · 2022-12-05T06:44:24Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

+    this.blocksToRetain = blocksToRetain;
+    this.pruningFrequency = pruningFrequency;
+    this.pruningExecutor =
+        new ThreadPoolExecutor(


Can this instead use the MonitoredExecutors.newFixedThreadPool so we have monitoring

Updated to use monitored executors.

jframe · 2022-12-05T06:44:48Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

+    this.blocksToRetain = blocksToRetain;
+    this.pruningFrequency = pruningFrequency;
+    this.pruningExecutor =
+        new ThreadPoolExecutor(


Rather the executor was passed in so that this isn't created in the constructor

jframe · 2022-12-05T06:46:43Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

+          if (blockNumber < pruningMark) {
+            // Ignore and warn if block number < pruning mark, this normally indicates the
+            // blocksToRetain
+            // is too small.


nit: comment could be moved to previous line // is too small.

Updated, I've removed unnecessary comments. This comment goes into the logging to show the user that the blocks to retain are too small (probably misconfiguration).

jframe · 2022-12-05T07:04:14Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/ProtocolContext.java

+
+  // TODO: cleanup - isChainPruningEnabled will not be required after
+  // https://github.com/hyperledger/besu/pull/4703 is merged.
+  public void setIsChainPruningEnabled(final boolean isChainPruningEnabled) {


Don't think this is a good place for the chain pruning enabled flag, it is very high level. Maybe part of the Merge context would be ok, as we only need this check for the Merge RPCs.

Updated. Ended up placing a flag in merge context since it involves very little change.

Co-authored-by: Simon Dudley <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]>

Signed-off-by: wcgcyx <[email protected]>

siladu · 2022-12-08T07:28:03Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

+              + " which normally indicates the blocksToRetain is too small");
+      return;
+    }
+    final KeyValueStorageTransaction tx1 = prunerStorage.startTransaction();


nit: do we need tx1 and tx2, any reason the same variable can't be reused?

If we do need two, then I think a more descriptive name would help readability

I was trying to use one variable actually but tx must be final in the lambda expression so couldn't be reused. Is there a way to get around that?

Gotcha, you could probably work around it with a class instead of lambda but it's not worth it for this. Renaming would be an improvement IMO but I wouldn't block the PR on it :)

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

Signed-off-by: wcgcyx <[email protected]>

siladu

Approved! Just the tx1, tx2 renaming suggestion but not a blocker for me

Signed-off-by: wcgcyx <[email protected]>

Signed-off-by: Zhenyang Shi <[email protected]>

jframe · 2022-12-09T06:28:15Z

CHANGELOG.md

@@ -10,6 +10,7 @@
 - Increase the speed of modexp gas execution and execution. [#4780](https://github.com/hyperledger/besu/pull/4780)
 - Added experimental CLI options `--Xeth-capability-max` and `--Xeth-capability-min` to specify a range of capabilities to be supported by the Eth protocol. [#4752](https://github.com/hyperledger/besu/pull/4752)
 - Set the default curve in the EVMTool, like is done in production operations [#4790](https://github.com/hyperledger/besu/pull/4790)
+- Add chain data pruning feature with three experimental CLI options: `--Xchain-data-pruning-enabled`, `--Xchain-data-pruning-blocks-retained` and `--Xchain-data-pruning-frequency` [#4686](https://github.com/hyperledger/besu/pull/4686)


nit: remove data from description based previous naming conversation

jframe · 2022-12-09T06:28:31Z

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/ChainDataPruningOptions.java

+
+import picocli.CommandLine;
+
+public class ChainDataPruningOptions implements CLIOptions<ChainPrunerConfiguration> {


rename to ChainPruningOPtions

jframe · 2022-12-09T06:33:14Z

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

@@ -315,6 +326,30 @@ public BesuController build() {
            blockchain, worldStateArchive, protocolSchedule, this::createConsensusContext);
    validateContext(protocolContext);

+    if (chainPrunerConfiguration.getChainPruningEnabled()) {
+      protocolContext.getConsensusContext(MergeContext.class).setIsChainPruningEnabled(true);


If we are not post-merge e.g. using QBFT then the merge context will not be available. You can use the ProtocolContext.safeConsensusContext to handle this case.

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java

jframe · 2022-12-09T06:38:34Z

ethereum/core/build.gradle

@@ -56,6 +56,7 @@ dependencies {
  implementation 'org.apache.tuweni:tuweni-rlp'
  implementation 'org.hyperledger.besu:bls12-381'
  implementation 'org.immutables:value-annotations'
+  implementation 'org.awaitility:awaitility'


This should only for testImplementation not implementation it's only needed for tests

yep, that's true, updated.

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

jframe · 2022-12-09T07:01:17Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

+          final KeyValueStorageTransaction pruningTransaction = prunerStorage.startTransaction();
+          long currentPruningMark = storedPruningMark;
+          final long newPruningMark = blockNumber - blocksToRetain;
+          if (event.isNewCanonicalHead()


I think this check for pruning frequency could be done before submitting it to the executor so that less work is done in case nothing needs to happen.

I need to think about this one.

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java

jframe · 2022-12-09T07:11:57Z

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainPrunerConfiguration.java

+    this.chainPruningBlocksRetained = chainPruningBlocksRetained;
+    this.chainPruningBlocksFrequency = chainPruningBlocksFrequency;
+  }
+


nit: chainPruning prefix could removed from the getters and fields are this is all on the ChainPrunerConfiguration class

Signed-off-by: wcgcyx <[email protected]>

* Add chain pruner * Increase minimum blocks to retain * Skip ancestor check in pruning mode * Separate class for pruning storage * Move pruning to separate thread * Limit total pruning threads Signed-off-by: wcgcyx <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Jason Frame <[email protected]> Signed-off-by: Karim TAAM <[email protected]>

* Add chain pruner * Increase minimum blocks to retain * Skip ancestor check in pruning mode * Separate class for pruning storage * Move pruning to separate thread * Limit total pruning threads Signed-off-by: wcgcyx <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Jason Frame <[email protected]> Signed-off-by: Ameziane H <[email protected]>

* Add chain pruner * Increase minimum blocks to retain * Skip ancestor check in pruning mode * Separate class for pruning storage * Move pruning to separate thread * Limit total pruning threads Signed-off-by: wcgcyx <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Jason Frame <[email protected]> Signed-off-by: Sally MacFarlane <[email protected]>

* Add chain pruner * Increase minimum blocks to retain * Skip ancestor check in pruning mode * Separate class for pruning storage * Move pruning to separate thread * Limit total pruning threads Signed-off-by: wcgcyx <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]> Co-authored-by: Simon Dudley <[email protected]> Co-authored-by: Jason Frame <[email protected]>

wcgcyx force-pushed the 4476-chain-data-pruning branch from 04f5c3b to fff4909 Compare November 16, 2022 08:38

wcgcyx added 6 commits November 18, 2022 15:00

Add chain pruner

fce33c1

Signed-off-by: wcgcyx <[email protected]>

Add tests and options

be8f081

Signed-off-by: wcgcyx <[email protected]>

Add pruning frequency

af59579

Signed-off-by: wcgcyx <[email protected]>

Fix unit tests

16de0e9

Signed-off-by: wcgcyx <[email protected]>

Fix chain pruning tests

3e67042

Signed-off-by: wcgcyx <[email protected]>

Increase minimum blocks to retain

0d433a3

Signed-off-by: wcgcyx <[email protected]>

wcgcyx force-pushed the 4476-chain-data-pruning branch from 1fd4fa6 to 0d433a3 Compare November 18, 2022 05:00

wcgcyx added 4 commits November 21, 2022 16:39

Skip ancestor check in pruning mode

0e545c5

Signed-off-by: wcgcyx <[email protected]>

Merge branch 'main' into 4476-chain-data-pruning

4e0e627

Signed-off-by: Zhenyang Shi <[email protected]>

Insert changelog entry & fix build issue

9313b02

Signed-off-by: wcgcyx <[email protected]>

Fix unit test

3e3596b

Signed-off-by: wcgcyx <[email protected]>

wcgcyx marked this pull request as ready for review November 21, 2022 08:47

Merge branch 'main' into 4476-chain-data-pruning

d0807ff

Signed-off-by: Zhenyang Shi <[email protected]>

usmansaleem reviewed Nov 21, 2022

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

usmansaleem reviewed Nov 21, 2022

View reviewed changes

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Outdated Show resolved Hide resolved

usmansaleem reviewed Nov 21, 2022

View reviewed changes

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/ChainDataPruningOptions.java Outdated Show resolved Hide resolved

usmansaleem reviewed Nov 21, 2022

View reviewed changes

besu/src/main/java/org/hyperledger/besu/controller/BesuControllerBuilder.java Outdated Show resolved Hide resolved

usmansaleem reviewed Nov 21, 2022

View reviewed changes

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/ChainDataPruner.java Outdated Show resolved Hide resolved

usmansaleem reviewed Nov 21, 2022

View reviewed changes

.../src/main/java/org/hyperledger/besu/ethereum/storage/keyvalue/KeyValueSegmentIdentifier.java Show resolved Hide resolved

Fix changelog & address reviews

4c1d792

Signed-off-by: wcgcyx <[email protected]>

wcgcyx requested a review from usmansaleem November 22, 2022 03:54

jframe reviewed Nov 23, 2022

View reviewed changes

wcgcyx added 3 commits November 24, 2022 13:03

Fix minor issues & code cleanup

95e86a5

Signed-off-by: wcgcyx <[email protected]>

Separate class for pruning storage

a63542a

Signed-off-by: wcgcyx <[email protected]>

Merge branch 'main' into 4476-chain-data-pruning

580942a

Signed-off-by: Zhenyang Shi <[email protected]>

siladu reviewed Nov 25, 2022

View reviewed changes

besu/src/main/java/org/hyperledger/besu/cli/options/unstable/ChainDataPruningOptions.java Outdated Show resolved Hide resolved

siladu reviewed Nov 25, 2022

View reviewed changes

besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java Show resolved Hide resolved

wcgcyx added 2 commits November 25, 2022 15:48

Rename options

328005b

Signed-off-by: wcgcyx <[email protected]>

Merge branch 'main' into 4476-chain-data-pruning

c34154f

Signed-off-by: Zhenyang Shi <[email protected]>

jframe reviewed Dec 5, 2022

View reviewed changes

wcgcyx and others added 9 commits December 6, 2022 12:42

Update besu/src/main/java/org/hyperledger/besu/cli/BesuCommand.java

cc4648e

Co-authored-by: Simon Dudley <[email protected]> Signed-off-by: Zhenyang Shi <[email protected]>

Spotless & Remove unnecessary comments

8315393

Signed-off-by: wcgcyx <[email protected]>

More clear variable

5992b66

Signed-off-by: wcgcyx <[email protected]>

Clear logging

5d4da4e

Signed-off-by: wcgcyx <[email protected]>

Fix bug & use monitored executors

118178e

Signed-off-by: wcgcyx <[email protected]>

New domain object

91102a7

Signed-off-by: wcgcyx <[email protected]>

Fix tests

962adb6

Signed-off-by: wcgcyx <[email protected]>

Move flag to merge context

1368c55

Signed-off-by: wcgcyx <[email protected]>

Merge branch 'main' into 4476-chain-data-pruning

d41661b

wcgcyx requested review from siladu and jframe December 8, 2022 01:38

siladu reviewed Dec 8, 2022

View reviewed changes

Renaming

2a439f1

Signed-off-by: wcgcyx <[email protected]>

wcgcyx force-pushed the 4476-chain-data-pruning branch from 88d6743 to 2a439f1 Compare December 9, 2022 02:44

siladu approved these changes Dec 9, 2022

View reviewed changes

wcgcyx added 2 commits December 9, 2022 15:37

Rename tx

d29a286

Signed-off-by: wcgcyx <[email protected]>

Merge branch 'main' into 4476-chain-data-pruning

e2c088c

Signed-off-by: Zhenyang Shi <[email protected]>

jframe reviewed Dec 9, 2022

View reviewed changes

Fix issues

25d514d

Signed-off-by: wcgcyx <[email protected]>

jframe approved these changes Dec 12, 2022

View reviewed changes

Merge branch 'main' into 4476-chain-data-pruning

571a022

macfarla merged commit 97588ae into hyperledger:main Dec 12, 2022

garyschulte mentioned this pull request Feb 24, 2023

propose Simon Dudley protocolguild/documentation#48

Merged

non-fungible-nelson mentioned this pull request Aug 21, 2023

Bonsai & Blockchain Storage Improvements #5387

Open

7 tasks

garyschulte mentioned this pull request Aug 31, 2023

Rocksdb Error when Downgrading from Besu 23.7.1 to 22.10.2 #5808

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add chain data pruning experimental feature #4686

Add chain data pruning experimental feature #4686

wcgcyx commented Nov 16, 2022 •

edited

Loading

jframe Nov 23, 2022

wcgcyx Nov 24, 2022

jframe Dec 4, 2022

wcgcyx Dec 4, 2022

wcgcyx Dec 5, 2022

jframe Dec 5, 2022

wcgcyx Dec 8, 2022

jframe Dec 5, 2022

wcgcyx Dec 6, 2022

jframe Dec 5, 2022

wcgcyx Dec 6, 2022

jframe Dec 5, 2022

wcgcyx Dec 6, 2022

jframe Dec 5, 2022

wcgcyx Dec 8, 2022

siladu Dec 8, 2022

siladu Dec 8, 2022

wcgcyx Dec 8, 2022

siladu Dec 9, 2022

siladu left a comment

jframe Dec 9, 2022

wcgcyx Dec 9, 2022

jframe Dec 9, 2022

wcgcyx Dec 9, 2022

jframe Dec 9, 2022

wcgcyx Dec 9, 2022

jframe Dec 9, 2022

wcgcyx Dec 9, 2022

jframe Dec 9, 2022

wcgcyx Dec 9, 2022

jframe Dec 9, 2022

wcgcyx Dec 9, 2022


		import picocli.CommandLine;

		public class ChainDataPruningOptions implements CLIOptions<ChainPrunerConfiguration> {

Add chain data pruning experimental feature #4686

Add chain data pruning experimental feature #4686

Conversation

wcgcyx commented Nov 16, 2022 • edited Loading

PR description

Fixed Issue(s)

Documentation

Changelog

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

siladu left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wcgcyx commented Nov 16, 2022 •

edited

Loading