Skip to content

feat(pruner, primitives): move prune batch sizes to ChainSpec#4318

Merged
shekhirin merged 35 commits intomainfrom
alexey/chain-spec-prune-batch-size
Aug 23, 2023
Merged

feat(pruner, primitives): move prune batch sizes to ChainSpec#4318
shekhirin merged 35 commits intomainfrom
alexey/chain-spec-prune-batch-size

Conversation

@shekhirin
Copy link
Member

@shekhirin shekhirin commented Aug 22, 2023

We need to set pruner batch sizes depending on the network the node is running. The reason for this is that mainnet has more transactions per block than testnets, hence more receipts, tx senders, tx lookup entries and changesets are inserted in the database. We need to prune more data than the amount of new incoming data, but also not too much because otherwise the problems described in #4246 will arise.

@shekhirin shekhirin changed the title feat(primitives): move prune batch sizes to ChainSpec feat(primitives): move prune batch sizes to ChainSpec Aug 22, 2023
@shekhirin shekhirin force-pushed the alexey/chain-spec-prune-batch-size branch from dd1f651 to 8c06a5d Compare August 22, 2023 15:48
@shekhirin shekhirin added the A-pruning Related to pruning or full node label Aug 22, 2023
@shekhirin shekhirin changed the title feat(primitives): move prune batch sizes to ChainSpec feat(pruner, primitives): move prune batch sizes to ChainSpec Aug 22, 2023
Comment on lines +55 to +61
/// These settings are sufficient to prune more data than generated with each new block.
pub const fn mainnet() -> Self {
Self {
receipts: 250,
transaction_lookup: 250,
transaction_senders: 250,
account_history: 1000,
Copy link
Member

Choose a reason for hiding this comment

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

assuming these are heuristics/made up?

Copy link
Member Author

Choose a reason for hiding this comment

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

  • receipts, transaction_lookup and transaction_senders – basically transactions per block which currently sits at 150 avg, occasionally jumping up to 250
  • account_historyAccountChangeSet table on avg grows for 200 entries per block, AccountHistory is cleared up based on the results of clearing the changesets
  • storage_historyStorageChangeSet table on avg grows for 500 entries per block, StorageHistory is cleared up based on the results of clearing the changesets

I just added a new DB metric #4316 which will allow to measure these values more accurately, as I was gathering the data just snapshotting the db stats

Copy link
Member

@gakonst gakonst left a comment

Choose a reason for hiding this comment

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

fine w me

Comment on lines +66 to +67
base_fee_params: BaseFeeParams::ethereum(),
prune_batch_sizes: PruneBatchSizes::mainnet(),
Copy link
Member

Choose a reason for hiding this comment

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

nice

Copy link
Collaborator

@joshieDo joshieDo left a comment

Choose a reason for hiding this comment

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

lgtm,

maybe it the future it can be self adaptive, since i guess it also depends on the machine running it

how did it perform on the last run?

  • did it start losing track of the tip? reduce batch size.
  • did it have "time to spare"? increase batch size

@shekhirin
Copy link
Member Author

shekhirin commented Aug 23, 2023

since i guess it also depends on the machine running it

I'm not sure there's a machine that can keep up with the tip and at the same time run the pruner for too long with current settings.

you're right, imagine it takes 5s to commit a new head and you also need to run the pruner in the time before the next FCU.

did it start losing track of the tip? reduce batch size.
did it have "time to spare"? increase batch size

yep, something like this should work. We can also be dependent on the number of transactions in the previous block for receipts/tx senders/tx lookup.

@shekhirin shekhirin force-pushed the alexey/chain-spec-prune-batch-size branch from 2f4561d to 2d40ea1 Compare August 23, 2023 11:13
@shekhirin shekhirin force-pushed the alexey/chain-spec-prune-batch-size branch from 005c3e1 to 3e511ca Compare August 23, 2023 17:09
@codecov
Copy link

codecov bot commented Aug 23, 2023

Codecov Report

Merging #4318 (623f3a5) into main (312cf72) will decrease coverage by 0.01%.
The diff coverage is 97.36%.

Impacted file tree graph

Files Changed Coverage Δ
bin/reth/src/node/mod.rs 11.81% <0.00%> (ø)
crates/config/src/config.rs 64.65% <0.00%> (ø)
crates/primitives/src/lib.rs 100.00% <ø> (ø)
crates/primitives/src/prune/mod.rs 87.17% <ø> (ø)
crates/consensus/beacon/src/engine/test_utils.rs 77.23% <100.00%> (ø)
crates/primitives/src/chain/spec.rs 96.55% <100.00%> (+0.01%) ⬆️
crates/primitives/src/prune/batch_sizes.rs 100.00% <100.00%> (ø)
crates/prune/src/pruner.rs 82.83% <100.00%> (-0.69%) ⬇️

... and 7 files with indirect coverage changes

Flag Coverage Δ
integration-tests 16.75% <19.73%> (-0.01%) ⬇️
unit-tests 63.86% <97.36%> (-0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Components Coverage Δ
reth binary 26.11% <0.00%> (ø)
blockchain tree 82.56% <ø> (ø)
pipeline 90.07% <ø> (ø)
storage (db) 74.71% <ø> (ø)
trie 94.88% <ø> (ø)
txpool 47.94% <ø> (ø)
networking 77.49% <ø> (-0.02%) ⬇️
rpc 58.81% <ø> (+<0.01%) ⬆️
consensus 63.53% <100.00%> (ø)
revm 31.97% <ø> (ø)
payload builder 6.78% <ø> (ø)
primitives 86.34% <100.00%> (+0.01%) ⬆️

Base automatically changed from alexey/pruner-batch-size to main August 23, 2023 17:34
@shekhirin shekhirin requested a review from rakita as a code owner August 23, 2023 17:34
@shekhirin shekhirin enabled auto-merge August 23, 2023 17:39
@shekhirin shekhirin added this pull request to the merge queue Aug 23, 2023
Merged via the queue into main with commit 1eee5ee Aug 23, 2023
@shekhirin shekhirin deleted the alexey/chain-spec-prune-batch-size branch August 23, 2023 17:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

A-pruning Related to pruning or full node

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants