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.

Sign up for GitHub

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

Jump to bottom

Default storage format now Bonsai #6536

Merged
merged 23 commits into from Feb 16, 2024
Merged

Default storage format now Bonsai #6536

merged 23 commits into from Feb 16, 2024

Conversation

macfarla
Copy link
Contributor

@macfarla macfarla commented Feb 7, 2024 •
edited

Builds on #6530
Change default storage format to BONSAI (was FOREST)

Note - if you had previously used FOREST, this is a breaking change - at startup you will get an error indicating the mismatch -

Mismatch: DB at '/your-path' is FOREST (Version 1) but config expects BONSAI (Version 2). Please check your config

Refs #5391

@macfarla
snap default
451d4d9 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
default bonsai
b0714e5 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
snap by default to replace fast
44c1425 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
default snap
7795f2c 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
removed dup test / merge snafu
5c8bce9 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
default bonsai
2f92139 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Merge branch 'main' of github.com:hyperledger/besu into bonsai-default
e6fa9cb
@macfarla
be explicit about config in tests
b8a6702 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Merge branch 'main' of github.com:hyperledger/besu into bonsai-default
872593a
@github-actions GitHub Actions
Copy link

github-actions bot commented Feb 7, 2024 •
edited by macfarla

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.
  • 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
  • I thought about running CI.
  • If I did not run CI, I ran as much locally as possible before pushing.

@macfarla
fixed test support in-memory storage
ece0c7c 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
default (forest) config for runner test
b321346 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
default (forest) config for ATs
a722135 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Merge branch 'main' of github.com:hyperledger/besu into bonsai-default
a23fec0
@macfarla
default forest storage for fast sync tests
8bb3c18 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Merge branch 'main' of github.com:hyperledger/besu into bonsai-default
a7638bb
@macfarla macfarla added the doc-change-required Indicates an issue or PR that requires doc to be updated label Feb 9, 2024
@macfarla macfarla mentioned this pull request Feb 13, 2024
Draft
@macfarla
Merge branch 'main' of github.com:hyperledger/besu into bonsai-default
c40de90
@macfarla
added upcoming breaking changes section
5fd4efd 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
changelog
79f5181 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@fab-10
Copy link
Contributor

fab-10 commented Feb 15, 2024

What happens if you are not specifying the format before and after the upgrade to this version, is the previous Forest format recognized or there is an format mismatch error?

@macfarla
Copy link
Contributor Author

What happens if you are not specifying the format before and after the upgrade to this version, is the previous Forest format recognized or there is an format mismatch error?

Good callout - Added to the description of the PR and the changelog

@macfarla
reword changelog
0824e50 
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla enabled auto-merge (squash) February 16, 2024 06:28
fab-10
fab-10 reviewed Feb 16, 2024
View reviewed changes
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

LGTM, just will prefer to have the tests use the new default instead of keeping using Forest

CHANGELOG.md Outdated Show resolved Hide resolved
.../dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ProcessBesuNodeRunner.java
params.addAll(
DataStorageOptions.fromConfig(
ImmutableDataStorageConfiguration.builder()
.from(DataStorageConfiguration.DEFAULT_FOREST_CONFIG)
Copy link
Contributor

Choose a reason for hiding this comment

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

should not be better to use BONSAI in tests since it is the default?

...s/dsl/src/main/java/org/hyperledger/besu/tests/acceptance/dsl/node/ThreadBesuNodeRunner.java
@@ -201,6 +202,7 @@ public void startNode(final BesuNode node) {
.nodeKey(new NodeKey(new KeyPairSecurityModule(KeyPairUtil.loadKeyPair(dataDir))))
.metricsSystem(metricsSystem)
.transactionPoolConfiguration(txPoolConfig)
.dataStorageConfiguration(DataStorageConfiguration.DEFAULT_FOREST_CONFIG)
Copy link
Contributor

Choose a reason for hiding this comment

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

same as above

@macfarla
Copy link
Contributor Author

That's a fair comment on the tests @fab-10 - I'm not going to get that done today though - do you reckon that could be a separate PR?

@fab-10
Copy link
Contributor

fab-10 commented Feb 16, 2024

That's a fair comment on the tests @fab-10 - I'm not going to get that done today though - do you reckon that could be a separate PR?

Yes. definitely it could be done in another PR

fab-10
fab-10 approved these changes Feb 16, 2024
View reviewed changes
Copy link
Contributor

@fab-10 fab-10 left a comment

Choose a reason for hiding this comment

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

LGTM, just the db mismatch warning message could be improved

@macfarla macfarla mentioned this pull request Feb 16, 2024
Open
@macfarla
Copy link
Contributor Author

That's a fair comment on the tests @fab-10 - I'm not going to get that done today though - do you reckon that could be a separate PR?

Yes. definitely it could be done in another PR

created #6583

macfarla and others added 2 commits February 16, 2024 20:25
@macfarla @fab-10
Update CHANGELOG.md
dcd3bae 
Co-authored-by: Fabio Di Fabio <fabio.difabio@consensys.net>
Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla
Merge branch 'main' into bonsai-default
d42908f
@macfarla macfarla merged commit 24f9201 into hyperledger:main Feb 16, 2024
50 checks passed
@macfarla macfarla mentioned this pull request Feb 18, 2024
Merged
suraneti pushed a commit to suraneti/besu that referenced this pull request Feb 24, 2024
@macfarla @suraneti
Default storage format now Bonsai (hyperledger#6536)
addc2ac 
* default bonsai
* be explicit about config in tests
* default forest storage for fast sync tests
* added upcoming breaking changes section
* added db mismatch message

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>

---------

Signed-off-by: Sally MacFarlane <macfarla.github@gmail.com>
@macfarla macfarla deleted the bonsai-default branch February 27, 2024 05:21
@alexandratran alexandratran removed the doc-change-required Indicates an issue or PR that requires doc to be updated label Feb 27, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@fab-10 fab-10 fab-10 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@macfarla @fab-10 @alexandratran