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

Introduce variables storage #5471

Merged
merged 17 commits into from
Jun 7, 2023
Merged

Conversation

fab-10
Copy link
Contributor

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

PR description

This is the first PR of a series to reorganize the data in db, in order to pave the way to future optimizations.
The first goal that we want to achieve is the optimization of the db for data that is static, leveraging the BlobDB feature.
For static data like blockchain or trie logs, BlobDB is a better choice since it reduces a lot the write amplification during compaction, with the result of faster initial sync and less wear out of the disk.

Now we have variable data mixed with static data in the blockchain column family, and this is not optimal, so this PR moved the variables into the dedicated column family VARIABLES that we can further optimize for frequently changing data in future.

The PR is meant automatically migrates existing variables to the new location on startup.

Logs during migration

{"@timestamp":"2023-06-06T10:14:15,156","level":"INFO","thread":"main","class":"KeyValueStoragePrefixedKeyBlockchainStorage","message":"Migrated key chainHeadHash to variables storage","throwable":""}
{"@timestamp":"2023-06-06T10:14:15,157","level":"INFO","thread":"main","class":"KeyValueStoragePrefixedKeyBlockchainStorage","message":"Migrated key finalizedBlockHash to variables storage","throwable":""}
{"@timestamp":"2023-06-06T10:14:15,158","level":"INFO","thread":"main","class":"KeyValueStoragePrefixedKeyBlockchainStorage","message":"Migrated key safeBlockHash to variables storage","throwable":""}
{"@timestamp":"2023-06-06T10:14:15,162","level":"INFO","thread":"main","class":"KeyValueStoragePrefixedKeyBlockchainStorage","message":"Migrated key forkHeads to variables storage","throwable":""}
{"@timestamp":"2023-06-06T10:14:15,163","level":"INFO","thread":"main","class":"KeyValueStoragePrefixedKeyBlockchainStorage","message":"Migrated key local-enr-seqno to variables storage","throwable":""}

It is possible to revert this change in case the user want to run a previous version of Besu, before downgrading the Beus, the user must run the subcommand storage revert-variables with the same configuration use to normally start Besu, and only after that it is possible to install a previous version of Besu.

Logs during revert

2023-06-06 10:15:33.201+00:00 | main | INFO  | DatabaseMetadata | Lookup database metadata file in data directory: /data/besu
2023-06-06 10:15:33.243+00:00 | main | INFO  | RocksDBKeyValueStorageFactory | Existing database detected at /data/besu. Version 2. Compacting database...
2023-06-06 10:15:33.772+00:00 | main | INFO  | StorageSubCommand$RevertVariablesStorage | Reverted variable storage for key chainHeadHash
2023-06-06 10:15:33.777+00:00 | main | INFO  | StorageSubCommand$RevertVariablesStorage | Reverted variable storage for key finalizedBlockHash
2023-06-06 10:15:33.777+00:00 | main | INFO  | StorageSubCommand$RevertVariablesStorage | Reverted variable storage for key safeBlockHash
2023-06-06 10:15:33.785+00:00 | main | INFO  | StorageSubCommand$RevertVariablesStorage | Reverted variable storage for key forkHeads
2023-06-06 10:15:33.786+00:00 | main | INFO  | StorageSubCommand$RevertVariablesStorage | Reverted variable storage for key local-enr-seqno

Fixed Issue(s)

prerequisite for #5475

@github-actions
Copy link

github-actions bot commented May 18, 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 variables-storage branch 2 times, most recently from 479dbc4 to 23cda5c Compare May 18, 2023 19:50
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>
@fab-10 fab-10 added the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 6, 2023
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 16:41
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
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.

Solid 👍 minor feedback

Bytes.wrap("safeBlockHash".getBytes(StandardCharsets.UTF_8));

private static final Logger LOG =
LoggerFactory.getLogger(KeyValueStoragePrefixedKeyBlockchainStorage.class);
private static final Bytes VARIABLES_PREFIX = Bytes.of(1);
Copy link
Contributor

Choose a reason for hiding this comment

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

perhaps rename to DEPRECATED_VARIABLES_PREFIX

Copy link
Contributor Author

Choose a reason for hiding this comment

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

added the Deprecated annotation

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

public static class Updater implements VariablesStorage.Updater {
Copy link
Contributor

Choose a reason for hiding this comment

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

Was there a need to have a separate updater apart from the blockchain storage updater?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it is possible to use the variables storage outside of the blockchain, like here, and will also be use to store the world state variables in future

Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
@fab-10 fab-10 enabled auto-merge (squash) June 7, 2023 08:17
@fab-10 fab-10 merged commit 8bc939d into hyperledger:main Jun 7, 2023
@fab-10 fab-10 deleted the variables-storage branch June 7, 2023 09:10
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Jun 20, 2023
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.

3 participants