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.

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

Trie log prune using TrieLogEvent #6394

Merged

Conversation

gfukushima
Copy link
Contributor

@gfukushima gfukushima commented Jan 12, 2024

PR description

This PR aims to make the pruning logic async as this feature does not need to be in the synchronous.

Fixed Issue(s)

Part of #5390

Testing

Tested locally on a small chain.

Tested with a fresh holesky sync...

TrieLogPruning happening on an EthSchedule-Services thread...

{"@timestamp":"2024-01-19T03:04:01,050","level":"TRACE","thread":"EthScheduler-Services-4","class":"TrieLogPruner","message":"adding trie log to queue for later pruning blockNumber 765656; blockHash 0xfd2e15c6ed1d00ee050210f427869d9ff6773555e5b738888fd9855b3251ad31","throwable":""}
{"@timestamp":"2024-01-19T03:04:01,050","level":"TRACE","thread":"EthScheduler-Services-4","class":"TrieLogPruner","message":"min((chainHeadNumber: 765655 - numBlocksToRetain: 512) = 765143, finalized: 765587)) = retainAboveThisBlockOrFinalized: 765143","throwable":""}
{"@timestamp":"2024-01-19T03:04:01,050","level":"TRACE","thread":"EthScheduler-Services-4","class":"TrieLogPruner","message":"pruned 1 trie logs for blocks {765143=[0x607bc6df9eabdb70a153dc23983a384c62cfd05a46a60584716cc4114c6c0ba3]}","throwable":""}
{"@timestamp":"2024-01-19T03:04:01,050","level":"DEBUG","thread":"EthScheduler-Services-4","class":"TrieLogPruner","message":"pruned 1 trie logs from 1 blocks","throwable":""}
{"@timestamp":"2024-01-19T03:04:01,052","level":"INFO","thread":"vert.x-worker-thread-0","class":"AbstractEngineNewPayload","message":"Imported #765,656 / 18 tx / 16 ws / base fee 1.04 gwei / 4,272,115 (14.2%) gas / (0xfd2e15c6ed1d00ee050210f427869d9ff6773555e5b738888fd9855b3251ad31) in 0.093s. Peers: 25","throwable":""}

Post sync, besu (on holesky) is only maintaining a single EthScheduler-Services thread so thread contention is low (it's a cachedThreadPool).
Screenshot 2024-01-19 at 1 06 04 pm

Tasks per second during and after sync...

Screenshot 2024-01-19 at 1 07 30 pm

Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Signed-off-by: Gabriel Fukushima <gabrielfukushima@gmail.com>
Copy link

github-actions bot commented Jan 12, 2024

  • 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

gfukushima and others added 3 commits January 12, 2024 18:02
Main change is to remove the unsafe prune of orphaned blocks
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu added the TeamGroot GH issues worked on by Groot Team label Jan 15, 2024
@siladu siladu self-assigned this Jan 15, 2024
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu marked this pull request as ready for review January 16, 2024 02:58
@Override
public void onTrieLogAdded(final TrieLogEvent event) {
if (TrieLogEvent.Type.ADDED.equals(event.getType())) {
final TrieLogAddedEvent addedEvent = (TrieLogAddedEvent) event;
Copy link
Contributor

Choose a reason for hiding this comment

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

This cast doesn't look necessary

Copy link
Contributor

@siladu siladu Jan 18, 2024

Choose a reason for hiding this comment

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

good spot, removed

@siladu
Copy link
Contributor

siladu commented Jan 17, 2024

remarking as draft while adding in the async piece

@siladu siladu marked this pull request as draft January 17, 2024 00:40
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu marked this pull request as ready for review January 18, 2024 11:39
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@@ -50,11 +53,13 @@ public class TrieLogPruner {
public TrieLogPruner(
final BonsaiWorldStateKeyValueStorage rootWorldStateStorage,
final Blockchain blockchain,
final Consumer<Runnable> executeAsync,
Copy link
Contributor

Choose a reason for hiding this comment

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

Might have been more obvious to pass EthScheduler in here but that would mean making the core module depend on the eth module which is circular.

I do like the way this is decoupled from the EthScheduler implementation anyway, even if the type is a little unobvious.

Copy link
Contributor

Choose a reason for hiding this comment

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

The Executor would be clearer than using a Consumer<Runnable> type

Copy link
Contributor

@siladu siladu Jan 22, 2024

Choose a reason for hiding this comment

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

As discussed, while it happens to be the same type, the implementation I'm using does not actually represent an Executor implementation (rather a delay execution of a submission to one 😅 )

Signed-off-by: Simon Dudley <simon.dudley@consensys.net>
@siladu siladu enabled auto-merge (squash) January 22, 2024 21:23
@siladu siladu merged commit 3fc3fb1 into hyperledger:main Jan 22, 2024
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
TeamGroot GH issues worked on by Groot Team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants