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

Don't save every state on archive node #210

Merged
merged 11 commits into from
Sep 27, 2023
Merged

Don't save every state on archive node #210

Changes from 5 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
b884ded
add skipping commiting states
magicxyyz Mar 29, 2023
c90d530
fix state commit skipping limits
magicxyyz Apr 11, 2023
5d2eddd
update MaxNumberOfBlocksToSkipStateSaving default value
magicxyyz Apr 11, 2023
c98e2c3
Merge branch 'recreate-state-for-rpcs' into dont-save-every-state
magicxyyz Jun 13, 2023
a47849f
fix merge
magicxyyz Jun 13, 2023
a20db86
Merge branch 'recreate-state-for-rpcs' into dont-save-every-state
magicxyyz Jun 15, 2023
eb96086
disable state save skipping in default geth cache config
magicxyyz Jun 15, 2023
c86303c
remove debug logs
magicxyyz Jun 15, 2023
1a58f1b
Merge branch 'master' into dont-save-every-state
magicxyyz Sep 7, 2023
0b3e0d0
Merge branch 'master' into dont-save-every-state
magicxyyz Sep 26, 2023
89f53b0
update flushing comment
magicxyyz Sep 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 37 additions & 3 deletions core/blockchain.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -144,6 +144,9 @@ type CacheConfig struct {
TriesInMemory uint64 // Height difference before which a trie may not be garbage-collected
TrieRetention time.Duration // Time limit before which a trie may not be garbage-collected

MaxNumberOfBlocksToSkipStateSaving uint32
MaxAmountOfGasToSkipStateSaving uint64

SnapshotNoBuild bool // Whether the background generation is allowed
SnapshotWait bool // Wait for snapshot construction on startup. TODO(karalabe): This is a dirty hack for testing, nuke it
}
Expand All @@ -153,8 +156,10 @@ type CacheConfig struct {
var defaultCacheConfig = &CacheConfig{

// Arbitrum Config Options
TriesInMemory: DefaultTriesInMemory,
TrieRetention: 30 * time.Minute,
TriesInMemory: DefaultTriesInMemory,
TrieRetention: 30 * time.Minute,
MaxNumberOfBlocksToSkipStateSaving: 127,
MaxAmountOfGasToSkipStateSaving: 15 * 1000 * 1000,
Copy link
Contributor

Choose a reason for hiding this comment

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

Defaults in geth should keep geth (ethereum) behavior as much as possible.
These two should definitely be 0.
Defaults in nitro can be debated (I tend for 0 there too but that's different)

Copy link
Collaborator

Choose a reason for hiding this comment

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

On the nitro side, I think trying to match the amount of gas inbetween saved states as L1 (15 million gas) would be good. Saving state for every Arbitrum block isn't as reasonable as it is for L1, but saving it every 15 million gas would mean that the rate of archive disk growth in terms of chain speed should be the same.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Then again, this would be something that'd be very easy to miss in an upgrade and would mean that the node is now somewhat permanently missing state. So maybe we should keep it at 0 for the upgrade, I'm not sure. We can talk about it next standup.


TrieCleanLimit: 256,
TrieDirtyLimit: 256,
Expand Down Expand Up @@ -236,6 +241,9 @@ type BlockChain struct {
processor Processor // Block transaction processor interface
forker *ForkChoice
vmConfig vm.Config

numberOfBlocksToSkipStateSaving uint32
amountOfGasInBlocksToSkipStateSaving uint64
}

type trieGcEntry struct {
Expand Down Expand Up @@ -1442,7 +1450,33 @@ func (bc *BlockChain) writeBlockWithState(block *types.Block, receipts []*types.
}
// If we're running an archive node, always flush
Copy link
Member

Choose a reason for hiding this comment

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

Probably good to update comment to reflect changed code

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've added a comment, I am open for any suggestions for improvement

if bc.cacheConfig.TrieDirtyDisabled {
return bc.triedb.Commit(root, false)
var maySkipCommiting, blockLimitReached, gasLimitReached bool
if bc.cacheConfig.MaxNumberOfBlocksToSkipStateSaving != 0 {
maySkipCommiting = true
if bc.numberOfBlocksToSkipStateSaving > 0 {
bc.numberOfBlocksToSkipStateSaving--
} else {
blockLimitReached = true
}
}
if bc.cacheConfig.MaxAmountOfGasToSkipStateSaving != 0 {
maySkipCommiting = true
if bc.amountOfGasInBlocksToSkipStateSaving >= block.GasUsed() {
bc.amountOfGasInBlocksToSkipStateSaving -= block.GasUsed()
} else {
gasLimitReached = true
}
}
if !maySkipCommiting || blockLimitReached || gasLimitReached {
bc.numberOfBlocksToSkipStateSaving = bc.cacheConfig.MaxNumberOfBlocksToSkipStateSaving
bc.amountOfGasInBlocksToSkipStateSaving = bc.cacheConfig.MaxAmountOfGasToSkipStateSaving
// TODO remove log
magicxyyz marked this conversation as resolved.
Show resolved Hide resolved
log.Debug("Saving state", "blockNumber", block.Number(), "blockHash", block.Hash(), "leftBlocks", bc.numberOfBlocksToSkipStateSaving, "leftGas", bc.amountOfGasInBlocksToSkipStateSaving)
return bc.triedb.Commit(root, false)
}
// TODO remove log
magicxyyz marked this conversation as resolved.
Show resolved Hide resolved
log.Debug("Skipping saving state", "blockNumber", block.Number(), "blockHash", block.Hash(), "leftBlocks", bc.numberOfBlocksToSkipStateSaving, "leftGas", bc.amountOfGasInBlocksToSkipStateSaving)
return nil
}

// Full but not archive node, do proper garbage collection
Expand Down