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.

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

Blobdb for static data #5475

Merged
merged 31 commits into from
Jun 7, 2023
Merged

Conversation

fab-10
Copy link
Contributor

@fab-10 fab-10 commented May 19, 2023

PR description

This PR is built on top of #5471, so check it first.
To only show the diff against #5471

This PR introduces optimizations for RocksDB column families that contain static data, i.e. key value entries that are never changed, but only added, like an append log storage, the best example is the blockchain storage, that benefit if BlobDB is used for them, since the write amplification is highly reduced, with the result of faster initial sync and less wear out of SSD.

Enabling BlobDB for a column family does not make the db incompatible with a previous version of Besu, however before downgrading Besu you need to run the subcommand storage revert-variables to revert changes done in PR #5471

Preliminary test shows that checkpoint sync is faster with BlobDB enabled, more tests are ongoing, and will share the results later.

Fixed Issue(s)

fixes #4607

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@github-actions
Copy link

github-actions bot commented May 19, 2023

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • I have considered running ./gradlew acceptanceTestNonMainnet locally if my PR affects non-mainnet modules.
  • I thought about the changelog and included a changelog update if required.
  • If my PR includes database changes (e.g. KeyValueSegmentIdentifier) I have thought about compatibility and performed forwards and backwards compatibility tests

return variables.get(key.toByteArray()).map(Bytes::wrap);
}

public static class Updater implements VariablesStorage.Updater {

Check notice

Code scanning / CodeQL

Class has same name as super class

Updater has the same name as its supertype [org.hyperledger.besu.ethereum.chain.VariablesStorage$Updater](1).
@fab-10 fab-10 force-pushed the blobdb-for-static-data branch 2 times, most recently from f944e0b to 93d5496 Compare May 19, 2023 11:40
Copy link
Contributor

@jflo jflo left a comment

Choose a reason for hiding this comment

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

So far so good. Have you considered re-using the DataStoreModule from evmtool? We might be able to pull that up into Besu, and use that to provide VariablesStorage to anything that needs it, via BesuComponent.

# Conflicts:
#	plugin-api/build.gradle
@fab-10 fab-10 self-assigned this May 30, 2023
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
# Conflicts:
#	ethereum/core/src/test-support/java/org/hyperledger/besu/ethereum/core/VariablesStorageHelper.java
@fab-10
Copy link
Contributor Author

fab-10 commented Jun 6, 2023

So far so good. Have you considered re-using the DataStoreModule from evmtool? We might be able to pull that up into Besu, and use that to provide VariablesStorage to anything that needs it, via BesuComponent.

Not analyzed that part, and given my little knowledge of that part, I will check with you what is necessary for that.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 marked this pull request as ready for review June 6, 2023 17:32
@garyschulte
Copy link
Contributor

the diff from 5471 looks good to me. I am not sure why the failing unit and acceptance tests.

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10
Copy link
Contributor Author

fab-10 commented Jun 7, 2023

the diff from 5471 looks good to me. I am not sure why the failing unit and acceptance tests.

unnecessary stubbing, fixed

@ahamlat
Copy link
Contributor

ahamlat commented Jun 7, 2023

Adding some metrics on this PR

Sync and block block processing timings

Sync Time
image

We can also see the difference in block import time, especially from some block number where the node running this PR has a much better block import time.

image image

Block processing (post sync)
Block processing time is very close between both nodes.

image

We can notice also the same performance regarding Forkchoice update (FCU)

image

System metrics

Block import time is better because with BlobDB there is less compaction and thus less write amplification.

Without PR #5475
image
We can notice below a huge IO activity with a lot of IO waits, 28 ms on average on writes.
image

With PR #5475
image
With this PR, there is less IO activity with less IO waits, 9 ms on average on writes.
image

Copy link
Contributor

@garyschulte garyschulte left a comment

Choose a reason for hiding this comment

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

🚢

@fab-10 fab-10 merged commit 9ae1b53 into hyperledger:main Jun 7, 2023
@fab-10 fab-10 deleted the blobdb-for-static-data branch June 7, 2023 17:07
elenduuche pushed a commit to elenduuche/besu that referenced this pull request Aug 16, 2023
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
eum602 pushed a commit to lacchain/besu that referenced this pull request Nov 3, 2023
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement and benchmark rocksdb BlobDB for blockchain storage
4 participants