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

Code storage delete refactor #6242

Closed
wants to merge 38 commits into from
Closed

Code storage delete refactor #6242

wants to merge 38 commits into from

Conversation

jframe
Copy link
Contributor

@jframe jframe commented Dec 5, 2023 •
edited
Loading

PR description

Note this is built on top of #5889. It will be much simpler once that is merged in.

Refactor BonsaiWorldStateKeyValueStorage to move the FlatDB construction and management out into a new class FlatDbStrategyProvider.

  • Splits out the FlatDbStrategy construction and flatDb management from the BonsaiWorldStateKeyValueStorage into a new class FlatDbStrategyProvider
  • Passes through the DataStorageConfiguration to FlatDbStrategyProvider instead of just the boolean flag
    By splitting out the FlatDbStrategy I can simplify the constructor arguments to the BonsaiWorldStateKeyValueStorage quite a bit as I can create the FlatDbStrategyProvider in the KeyValueStorageProvider once with its arguments instead of repassing those into the BonsaiWorldStateKeyValueStorage each time we create an instance of it.

Fixed Issue(s)

jframe added 30 commits October 6, 2023 13:57
@jframe
Code storage by keyed by code hash
f68da4e 
Signed-off-by: Jason Frame <[email protected]>
@jframe
remove unused function
548a2f8 
Signed-off-by: Jason Frame <[email protected]>
@jframe
code storage legacy tests
0aa8ff9 
Signed-off-by: Jason Frame <[email protected]>
@jframe
code storage unit tests and Byte32 -> Hash refactor
1e803e5 
Signed-off-by: Jason Frame <[email protected]>
@jframe
add get method to SegmentedKeyValueStorageTransaction
30ee2d7 
Signed-off-by: Jason Frame <[email protected]>
@jframe
use get method on transaction so that pending updates update correct …
ec862a6 
…count for code count updates

Signed-off-by: Jason Frame <[email protected]>
@jframe
unit tests for BonsaiWorldState updateCode
2413afd 
Signed-off-by: Jason Frame <[email protected]>
@jframe
fix compile error on PersistDataStepTest
2f051ce 
Signed-off-by: Jason Frame <[email protected]>
@jframe
fix early return in BonsaiWorldState
b1465d5 
Signed-off-by: Jason Frame <[email protected]>
@jframe
prevent any updates when code hasn't changed to ensure count is not i…
9117771 
…ncremented unnecessarily

Signed-off-by: Jason Frame <[email protected]>
@jframe
additional tests for putCode in BonsaiWorldState
27bab00 
Signed-off-by: Jason Frame <[email protected]>
@jframe
only store code value if it doesn't already exist
387d8e4 
Signed-off-by: Jason Frame <[email protected]>
@jframe
change remove to delete code hash count when reaching zero references
676344d 
Signed-off-by: Jason Frame <[email protected]>
@jframe
rename code storage strategies classes to clearer names
67373b6 
Signed-off-by: Jason Frame <[email protected]>
@jframe
renamed tests to make it clear they are related to updating code storage
3578687 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Merge remote-tracking branch 'upstream/main' into code_storage_by_hash
0353a30
@jframe
update plugin-api hash
99aad96 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Merge remote-tracking branch 'upstream/main' into code_storage_by_hash
bbb530e
@jframe
Use same column family for code storage using code hash and introduce…
4046fc8 
… feature flag --Xsnapsync-server-enabled

Signed-off-by: Jason Frame <[email protected]>
@jframe
Remove TODOs in InMemoryKeyValueStorageProvider as this is only used …
761b4a2 
…in tests

Signed-off-by: Jason Frame <[email protected]>
@jframe
apply spotless
14b8e2d 
Signed-off-by: Jason Frame <[email protected]>
@jframe
fix tests
ec73ac2 
Signed-off-by: Jason Frame <[email protected]>
@jframe
retrieve code by hash using prefix so it works with same column family
2ee7cb3 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Use VariablesKeyValueStorage for code hash storage mode
0d6c7f9 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Clean up removing unneeded code
44e0ca5 
Signed-off-by: Jason Frame <[email protected]>
@jframe
tests
ce175c2 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Merge branch 'main' into code_storage_by_hash
ffa98f9
@jframe
Removing unused composed world state storage
f2bbb99 
Signed-off-by: Jason Frame <[email protected]>
@jframe
Merge branch 'code_storage_by_hash' of github.com:jframe/besu into co…
50aac96 
…de_storage_by_hash
@jframe
Change feature flag to --Xbonsai-code-using-code-hash-enabled
d023568 
Signed-off-by: Jason Frame <[email protected]>
@jframe jframe added the TeamGroot GH issues worked on by Groot Team label Dec 5, 2023
@github-actions GitHub Actions
Copy link

github-actions bot commented Dec 5, 2023

  • 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

@jframe jframe self-assigned this Dec 5, 2023
matkt
matkt reviewed Dec 11, 2023
View reviewed changes
Copy link
Contributor

@matkt matkt 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 not sure about a variable name

ethereum/core/src/main/java/org/hyperledger/besu/ethereum/chain/VariablesStorage.java
@@ -29,7 +29,8 @@ enum Keys {
FORK_HEADS("forkHeads"),
FINALIZED_BLOCK_HASH("finalizedBlockHash"),
SAFE_BLOCK_HASH("safeBlockHash"),
SEQ_NO_STORE("local-enr-seqno");
SEQ_NO_STORE("local-enr-seqno"),
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure this name is clear enough

@jframe jframe closed this Jan 15, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@matkt matkt matkt left review comments

Assignees

@jframe jframe

Labels
TeamGroot GH issues worked on by Groot Team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@jframe @matkt