Skip to content

Conversation

matkt
Copy link
Contributor

@matkt matkt commented May 15, 2020

Signed-off-by: Karim TAAM karim.t2am@gmail.com

PR description

Fix RocksDBException during sweeping

The issue comes from an error in the closing of WriteOptions (RocksDBColumnarKeyValueStorage)

When we call new WriteOptions() this will create a structure https://github.com/influxdata/rocksdb/blob/7adff3eb33001c471c48cdcadf1b55462920b123/options.go#L64

When we call WriteOptions->close() this will free the structure created
https://github.com/influxdata/rocksdb/blob/7adff3eb33001c471c48cdcadf1b55462920b123/options.go#L242

The test reveals a bug regarding the calls to WriteOptions close() methods

Scenario

1 - Create a WriteOption for the RocksDBColumnarKeyValueStorage instance (optionA)
final SegmentedKeyValueStorage<ColumnFamilyHandle> store = createSegmentedStore();

2 - Create a new transaction with a new WriteOption specific to it (optionB)
final Transaction<ColumnFamilyHandle> tx = store.startTransaction();

3 - Commits the transaction and releases the wrong instance of WriteOption (optionA+optionB)
tx.commit();

4 - Call the tryDelete method which uses the WriteOption instance of the RocksDbTransaction class. Problem this instance of WriteOption has been released and therefore may have random values
store.tryDelete(fooSegment, key)

How to fix

Do not release the WriteOption (optionA) during a transaction commit but when the RocksDBColumnarKeyValueStorage instance is closed

Adding a test that performed more than 1000 times the tests and it works. Without the fix it often fails very quickly

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
@matkt matkt linked an issue May 15, 2020 that may be closed by this pull request
@matkt matkt marked this pull request as ready for review May 15, 2020 15:21
Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Copy link
Contributor

@RatanRSur RatanRSur left a comment

Choose a reason for hiding this comment

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

Great job! Thanks for looking into this.

@RatanRSur RatanRSur merged commit 5f92f54 into hyperledger:master May 15, 2020
@RatanRSur RatanRSur deleted the feature/besu-888-fix-rocksdb-Exception-during-sweeping branch May 15, 2020 16:40
shemnon pushed a commit to shemnon/besu that referenced this pull request May 23, 2020
Co-authored-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Danno Ferrin <danno.ferrin@gmail.com>
macfarla pushed a commit to macfarla/besu that referenced this pull request Jun 3, 2020
Co-authored-by: Ratan Rai Sur <ratan.r.sur@gmail.com>

Signed-off-by: Karim TAAM <karim.t2am@gmail.com>
Signed-off-by: Sally MacFarlane <sally.macfarlane@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.

"Sync Writes has to enable WAL" RocksDBException during sweeping
2 participants