-
Notifications
You must be signed in to change notification settings - Fork 844
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
Default bonsai to fully flat db and code storage by codehash #6894
Default bonsai to fully flat db and code storage by codehash #6894
Conversation
7c12480
to
d201d64
Compare
It is worth noting there is a side effect of this PR on worldstate fault heal/recovery. If a db with a We can re-enable flat db after the healing, but this side effect might actually be desirable. |
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.
added small comments
@@ -217,6 +217,7 @@ receipt-compaction-enabled=true | |||
# feature flags | |||
Xsecp256k1-native-enabled=false | |||
Xaltbn128-native-enabled=false | |||
Xsnapsync-server-enabled=true |
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.
we should not add the new full flat db flag ?
@@ -152,6 +152,8 @@ public void assertCloseDisposesOfStateWithoutCommitting() { | |||
assertThat(res.isSuccessful()).isTrue(); | |||
shouldCloseSnapshot.persist(oneTx.getHeader()); | |||
|
|||
// TODO: full flat strategy will not be updated with these values, only the trie | |||
// determine how we want to make this assertion for full flat |
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 think we can use an updater in order to fix the test
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 thought about it and realized it will degrade the performance of the 'normal' use case for frozen/cached worldstates. I think this note in the test superclass should be sufficient to explain why we are using PARTIAL for the test - removed from TODO.
var flatDbMode = | ||
FlatDbMode.fromVersion( | ||
composedWorldStateStorage | ||
.get(TRIE_BRANCH_STORAGE, FLAT_DB_MODE) | ||
.map(Bytes::wrap) | ||
.orElse(FlatDbMode.PARTIAL.getVersion())); | ||
.orElseGet( |
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.
👍
@@ -71,12 +72,36 @@ public void loadFlatDbStrategy(final SegmentedKeyValueStorage composedWorldState | |||
|
|||
@VisibleForTesting | |||
FlatDbMode deriveFlatDbStrategy(final SegmentedKeyValueStorage composedWorldStateStorage) { | |||
final FlatDbMode requestedFlatDbMode = | |||
dataStorageConfiguration.getUnstable().getBonsaiFullFlatDbEnabled() |
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.
is this flag still in unstable ?
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.
it is, but the generated classes have to be rebuilt to see it. Pretty much all of the bonsai configs are in unstable, we should move them all out of unstable when we change the default to bonsai 👍
db32129
to
b16d74d
Compare
@@ -183,16 +183,6 @@ public CompletableFuture<Void> run( | |||
} else { | |||
// start from scratch | |||
worldStateStorageCoordinator.clear(); | |||
// we have to upgrade to full flat db mode if we are in bonsai mode | |||
if (snapSyncConfiguration.isFlatDbHealingEnabled()) { |
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.
we don't have to downgradeToPartialFlatDbMode if a user disable the full flat flag ? as the default is full now
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.
that is a good point. I hadn't considered that anybody would want to run in a partial mode. I will double check, but the storage should be created in the correct/requested mode, whether that is partial or full.
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.
We can prevent user from doing that. It would not be logic
b16d74d
to
11a8d25
Compare
Signed-off-by: garyschulte <[email protected]>
…torageOption for --Xbonsai-full-flat-db-enabled Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
Signed-off-by: garyschulte <[email protected]>
11a8d25
to
f7055ec
Compare
Signed-off-by: garyschulte <[email protected]>
…dger#6894) * change to full flat db and code stored by code hash by default * deprecate --Xsnapsync-synchronizer-flat-db-healing-enabled, add DataStorageOption for --Xbonsai-full-flat-db-enabled Signed-off-by: garyschulte <[email protected]> Signed-off-by: Justin Florentine <[email protected]>
…dger#6894) * change to full flat db and code stored by code hash by default * deprecate --Xsnapsync-synchronizer-flat-db-healing-enabled, add DataStorageOption for --Xbonsai-full-flat-db-enabled Signed-off-by: garyschulte <[email protected]>
…dger#6894) * change to full flat db and code stored by code hash by default * deprecate --Xsnapsync-synchronizer-flat-db-healing-enabled, add DataStorageOption for --Xbonsai-full-flat-db-enabled Signed-off-by: garyschulte <[email protected]> Signed-off-by: Justin Florentine <[email protected]>
PR description
This PR changes the default storage mode of bonsai to use code storage by code hash, and uses flat db by default. This supports the future direction of bonsai, and simplifies the configuration necessary for besu to serve snap protocol (see description on #6640, now just
--Xsnapsync-server-enabled
is necessary).changes:
tested with:
Fixed Issue(s)
Thanks for sending a pull request! Have you done the following?
doc-change-required
label to this PR if updates are required.Locally, you can run these tests to catch failures early:
./gradlew build
./gradlew acceptanceTest
./gradlew integrationTest
./gradlew ethereum:referenceTests:referenceTests