Rocksdb plugin to support OptimisticTransactionDb and TransactionDb #5328

gfukushima · 2023-04-11T11:45:32Z

PR description

Tis PR introduces some changes to the RocksDB plugin to allow using the DB in either pessimistic or optimistic transaction mode.
Currently pessimistic is tied to the usage of Forest data format.

The refactor was done with the intent to keep bonsai as it is currently with usage of the optimistic transaction db.

This is just a initial draft not ready for review yet.

Fixed Issue(s)

Fixes #5300

Scenarios tested:

Sepolia

Fast-sync Forest (uses Pessimistic mode)
Snap-sync Bonsai (uses Optimistic mode)

Goerli

Fast-sync Forest (uses Pessimistic mode)
Snap-sync Bonsai (uses Optimistic mode)

Mainnet

Snap-sync Forest (uses Pessimistic mode) not finished yet
Snap-sync Bonsai (uses Optimistic mode)
Checkpoint-sync Forest (uses Pessimistic mode) not finished yet
Checkpoint-sync Bonsai (uses Optimistic mode)

Backwards compatibility

Roll back a Forest synced node to 23.1.2 forcing it to use Optimistic
Update a current synced Forest node that is using Optimistic to use the new version with pessimistic to ensure this won't require resyncs. (not done yet)

…t to optimistic and pessimistic Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

github-actions · 2023-04-11T11:45:43Z

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.

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java

...esu/plugin/services/storage/rocksdb/segmented/PessimisticRocksDBColumnarKeyValueStorage.java

...besu/plugin/services/storage/rocksdb/segmented/OptimisticRocksDBColumnarKeyValueStorage.java

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe · 2023-04-13T01:44:39Z

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java

+              LOG.info("FOREST mode detected, using pessimistic DB.");
+              segmentedStorage =
+                  new PessimisticRocksDBColumnarKeyValueStorage(
+                      rocksDBConfiguration,
+                      segmentsForVersion,
+                      ignorableSegments,
+                      metricsSystem,
+                      rocksDBMetricsFactory);


This could be done just for databaseVersion 1 in the switch statement as that indicates the db format is forest. Then the isForestStorageFormat wouldn't be needed either.

I created that flag on purpose cause it took me some time to understand that DBversion actually is related to forest or bonsai. I think the usage of numbers to identify version is fine. But giving some more context to the code so next devs to touch don't spend time trying to understand what this version means.
In my first draft I wasted some time piping the StorageFormat through the plugin API because I didn't know that database version represented that same information.

...esu/plugin/services/storage/rocksdb/segmented/PessimisticRocksDBColumnarKeyValueStorage.java

non-fungible-nelson · 2023-04-14T14:38:10Z

Can you explain the benefits of these modes/this PR?

I think a review from @garyschulte at some point be useful as well (not urgent prior to the next release).

garyschulte · 2023-04-14T14:46:33Z

The main benefit is that it simplifies concurrency for forest mode. Bonsai relies on Snapshots, which require OptimisticTransactionDb.

For some reason Optimistic causes transaction locking timeouts for forest mode. We could spend time on forest to discover why, but if we can avoid the issue by reverting toTransactionDb instead, that is a win.

I think there is value in having the ability to configure both modes with the rocksdb plugin, so that neither storage format forces the other mode to use a specific transaction mode.

gfukushima · 2023-04-14T22:43:40Z

@non-fungible-nelson It is pretty much impossible to sync a node currently using Forest (fast/snap/checkpoint). The worldstate(ws) download pipeline gets a lot of locking timeouts like Gary said and halts. We tried different approachs to avoid the ws pipeline to break (mostly some retries logics). Eventually switching Forest back to Pessimistic mode puts Forest back on track to get the the initial sync working.

gfukushima · 2023-04-14T22:47:36Z

I think there is value in having the ability to configure both modes with the rocksdb plugin, so that neither storage format forces the other mode to use a specific transaction mode.

@garyschulte
I actually thought about introducing a new flag at the plugin level since this is actually related to the plugin itself and not the stoage-format we're using but eventually this would allow users to make no-functional combinations such as bonsai and pessimistic (which we probably don't want to).
We could possibly tie the TxDB mode with data-storage-format at the CLI level with some flags validations.

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java

garyschulte

LGTM, minor suggestions about naming and adapter split.

...ger/besu/plugin/services/storage/rocksdb/unsegmented/RocksDBColumnarKeyValueStorageTest.java

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java

...esu/plugin/services/storage/rocksdb/segmented/PessimisticRocksDBColumnarKeyValueStorage.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

…actionDB Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

macfarla

javadoc - needs a find/replace for pessimistic

...u/plugin/services/storage/rocksdb/segmented/TransactionDBRocksDBColumnarKeyValueStorage.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

...besu/plugin/services/storage/rocksdb/segmented/OptimisticRocksDBColumnarKeyValueStorage.java

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

garyschulte

LGTM, just one more nitpick about class naming

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java

.../java/org/hyperledger/besu/services/kvstore/NonSnappableSegmentedKeyValueStorageAdapter.java

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

garyschulte

🚢

# Conflicts: # CHANGELOG.md

…yperledger#5328) * Refactor to make RocksDBColumnarKeyValueStorage abstract and extend it to optimistic and pessimistic. Atm Forest has a concurrency level that does not cope well with the OptimisticTransactionDB and RocksDB ends up raising a lot of Busy when committing RocksDBTransactions. A TransactionDB will do conflict checking for all write operations (Put, Delete and Merge), including writes performed outside a Transaction according to RocksDB. This does impact the times we see when syncing so likely not the final solution for Forest. Bonsai should continue using OptimisticTransactionDB. Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com> --------- Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima added 4 commits April 11, 2023 21:12

Refactor to make RocksDBColumnarKeyValueStorage abstract and extend i…

bf3b9f8

…t to optimistic and pessimistic Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Refactor to use RocksDB class where possible

a6062c3

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Replace RocksDBColumnarKeyValueStorage for equivalent use case

9b7d68e

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add config to identify storage mode

de371a9

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima changed the title ~~Rocksdb plugin to support OptimisticTxDb and PessimisticTxDb~~ Rocksdb plugin to support OptimisticTransactionDb and TransactionDb Apr 11, 2023

github-advanced-security bot found potential problems Apr 11, 2023

View reviewed changes

gfukushima added 4 commits April 12, 2023 14:02

javadoc

93322ea

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Removing unnecessary addition to plugin api

46be43f

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Refactor to remove duplicated code

11c8a61

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Introduce concept of DataStorageFormat.FOREST to the rocksDB plugin

f8e848e

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

github-advanced-security bot found potential problems Apr 12, 2023

View reviewed changes

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java Fixed Show fixed Hide fixed

...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java Fixed Show fixed Hide fixed

gfukushima added TeamGroot GH issues worked on by Groot Team mainnet labels Apr 12, 2023

gfukushima self-assigned this Apr 12, 2023

gfukushima added 2 commits April 13, 2023 12:44

Removing code added during spike

42e7924

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'main' into rocksdb-to-support-opTxDb-pessimisticTxDB

c230472

jframe reviewed Apr 13, 2023

View reviewed changes

gfukushima marked this pull request as ready for review April 14, 2023 08:05

garyschulte reviewed Apr 15, 2023

View reviewed changes

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java Outdated Show resolved Hide resolved

garyschulte reviewed Apr 15, 2023

View reviewed changes

gfukushima mentioned this pull request Apr 17, 2023

Remove RocksDB option maxBackgroundCompactions deprecated in version 8.0 #5351

Closed

gfukushima added 4 commits April 17, 2023 13:39

Moving to the segmented folder since this is segmented DB tests

c223ab5

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Remove takeSnapshot method from abstract and pessimistic class

c1c6bce

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Rename class and remove takeSnapshot method since it's not supported

aaba2bc

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Make takeSnapshot a public method of the class

4e24937

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima added 6 commits April 17, 2023 14:29

Use nonSnappableAdapter for forest segmented storage

ebda4cf

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Add NonSnappableSegmentedKeyValueStorageAdapter for forest

3238f97

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

spdx

32adbf6

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Extend RocksDBColumnarKeyValueStorageTest to use Optimistic and Trans…

a0cdd20

…actionDB Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

spdx

298ab61

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'main' into rocksdb-to-support-opTxDb-pessimisticTxDB

894dfb8

macfarla reviewed Apr 17, 2023

View reviewed changes

...u/plugin/services/storage/rocksdb/segmented/TransactionDBRocksDBColumnarKeyValueStorage.java Outdated Show resolved Hide resolved

...u/plugin/services/storage/rocksdb/segmented/TransactionDBRocksDBColumnarKeyValueStorage.java Outdated Show resolved Hide resolved

javadoc rename fix

2076716

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

gfukushima requested a review from garyschulte April 17, 2023 06:27

jframe reviewed Apr 17, 2023

View reviewed changes

gfukushima added 3 commits April 18, 2023 10:42

Clean up code

bc4e3fc

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

Merge branch 'main' into rocksdb-to-support-opTxDb-pessimisticTxDB

6ed4df6

changelog

80b9470

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

jframe approved these changes Apr 18, 2023

View reviewed changes

garyschulte approved these changes Apr 18, 2023

View reviewed changes

...java/org/hyperledger/besu/plugin/services/storage/rocksdb/RocksDBKeyValueStorageFactory.java Outdated Show resolved Hide resolved

garyschulte reviewed Apr 18, 2023

View reviewed changes

.../java/org/hyperledger/besu/services/kvstore/NonSnappableSegmentedKeyValueStorageAdapter.java Outdated Show resolved Hide resolved

Use SegmentedKeyValueStorageAdapter as base class for Snappable adapter

7cee988

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>

garyschulte approved these changes Apr 19, 2023

View reviewed changes

Merge branch 'main' into rocksdb-to-support-opTxDb-pessimisticTxDB

df8a0aa

# Conflicts: # CHANGELOG.md

gfukushima merged commit d54a1bf into hyperledger:main Apr 19, 2023

gfukushima mentioned this pull request Apr 21, 2023

Investigate and fix Hive engine-api tests #5298

Closed

This was referenced Jun 13, 2023

org.rocksdb.RocksDBException on Fast Sync #3630

Closed

Error When I try to fast syncsync #4369

Closed

Besu world state download pipeline cancelled due to RocksDB Busy exception. #4901

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rocksdb plugin to support OptimisticTransactionDb and TransactionDb #5328

Rocksdb plugin to support OptimisticTransactionDb and TransactionDb #5328

gfukushima commented Apr 11, 2023 •

edited

Loading

github-actions bot commented Apr 11, 2023 •

edited by gfukushima

Loading

jframe Apr 13, 2023

gfukushima Apr 13, 2023

non-fungible-nelson commented Apr 14, 2023

garyschulte commented Apr 14, 2023

gfukushima commented Apr 14, 2023 •

edited

Loading

gfukushima commented Apr 14, 2023

garyschulte left a comment

macfarla left a comment

garyschulte left a comment

garyschulte left a comment

Rocksdb plugin to support OptimisticTransactionDb and TransactionDb #5328

Rocksdb plugin to support OptimisticTransactionDb and TransactionDb #5328

Conversation

gfukushima commented Apr 11, 2023 • edited Loading

PR description

Fixed Issue(s)

github-actions bot commented Apr 11, 2023 • edited by gfukushima Loading

jframe Apr 13, 2023

Choose a reason for hiding this comment

gfukushima Apr 13, 2023

Choose a reason for hiding this comment

non-fungible-nelson commented Apr 14, 2023

garyschulte commented Apr 14, 2023

gfukushima commented Apr 14, 2023 • edited Loading

gfukushima commented Apr 14, 2023

garyschulte left a comment

Choose a reason for hiding this comment

macfarla left a comment

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

garyschulte left a comment

Choose a reason for hiding this comment

gfukushima commented Apr 11, 2023 •

edited

Loading

github-actions bot commented Apr 11, 2023 •

edited by gfukushima

Loading

gfukushima commented Apr 14, 2023 •

edited

Loading