-
Notifications
You must be signed in to change notification settings - Fork 837
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 RocksDbSegmentIdentifier to avoid changing the storege plug… #3755
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Love it. Feedback on naming, but nothing blocking
import org.rocksdb.RocksDBException; | ||
import org.rocksdb.TransactionDB; | ||
|
||
public class RocksDbSegmentIdentifier { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how about RocksDbColumnFamilyIdentifier instead?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Used SegmentedIdentifier to follow the naming used in the interface, but I do not have a strong opinion on this, so if you think RocksDbColumnFamilyIdentifier
is fits better, I can rename it
this.reference = new AtomicReference<>(columnFamilyHandle); | ||
} | ||
|
||
public void recreate() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't think recreate implies storage is reclaimed. how about reset or clear?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reset
sound good
…in interface Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
...perledger/besu/plugin/services/storage/rocksdb/segmented/RocksDBColumnarKeyValueStorage.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Kudos, SonarCloud Quality Gate passed! |
hyperledger#3755) * Introduce RocksDbSegmentIdentifier to avoid changing the storege plugin interface Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net> Signed-off-by: Lucas Saldanha <lucascrsaldanha@gmail.com>
hyperledger#3755) * Introduce RocksDbSegmentIdentifier to avoid changing the storege plugin interface Signed-off-by: Fabio Di Fabio <fabio.difabio@consensys.net>
…in interface
Signed-off-by: Fabio Di Fabio fabio.difabio@consensys.net
PR description
A refactor of #3734 to avoid changing the storage plugin interface
Fixed Issue(s)
fixes #3722
Documentation
doc-change-required
label to this PR ifupdates are required.
Changelog