-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Pruning #101
Pruning #101
Conversation
b00ris
commented
Sep 30, 2019
- added async pruning
cmd/utils/flags.go
Outdated
} | ||
GCModeTickTimeout = cli.DurationFlag{ | ||
Name: "gcmode.tick", | ||
Usage: `Block to prune per tick"`, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Usage value needs to be changed
core/pruner.go
Outdated
|
||
func batchDelete(db *bolt.DB, keys *keysToRemove) error { | ||
log.Debug("Removed: ", "accounts", len(keys.Account), "storage", len(keys.Storage), "suffix", len(keys.Suffix)) | ||
return db.Update(func(tx *bolt.Tx) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this may be happening concurrently with other updates, we might consider using db.Batch() in both places (then boltdb will be able to merge concurrent updates sometimes). But the requirement is that all updates need to be idempotent (have the same effect if repeated multiple times)
core/pruner.go
Outdated
"time" | ||
) | ||
|
||
const NumOfPrunedBlocks = 10 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dead code
core/pruner.go
Outdated
if !ok { | ||
return errors.New("it's not ethdb.BoltDatabase") | ||
} | ||
if p.config.BlocksToPrune == 0 || p.config.PruneTimeout.Seconds() < 1 { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sanitise this at configuration stage
core/pruner.go
Outdated
|
||
db ethdb.Database | ||
chain BlockChainer | ||
LastPrunedBlockNum *big.Int |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can use uint64 and atomic, to get rid of mutex
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As we discussed, this does not even need to be protected, it is only used in one goroutine
core/pruner.go
Outdated
continue | ||
} | ||
p.RLock() | ||
numOfBlocks := calculateNumOfPrunedBlocks(cb.Number().Uint64(), p.LastPrunedBlockNum.Uint64(), p.config.BlocksBeforePruning, p.config.BlocksToPrune) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That function can compute from
and to
core/pruner.go
Outdated
|
||
changedKeys := dbutils.Suffix(v) | ||
|
||
err := changedKeys.Walk(func(addrHash []byte) error { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
addrHash
here could mean not just address hash, but also concatenation of address hash and the key hash, right? Consider renaming it for clarity
core/pruner.go
Outdated
err := changedKeys.Walk(func(addrHash []byte) error { | ||
compKey, _ := dbutils.CompositeKeySuffix(addrHash, timestamp) | ||
ck := make([]byte, len(compKey)) | ||
copy(ck, compKey) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the function CompositeKeySuffix
already allocates a new slice for the key, this is not needed, wasteful extra allocation. Please remove if so
|
||
func Prune(db *ethdb.BoltDatabase, blockNumFrom uint64, blockNumTo uint64) error { | ||
keysToRemove := newKeysToRemove() | ||
err := db.Walk(dbutils.SuffixBucket, []byte{}, 0, func(key, v []byte) (b bool, e error) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add the limit to how large the keysToRemove
can become. For example, no more than 100k items. Otherwise, bolt transaction is going to be too large, and the whole program can crash with Out Of Memory
…ployer [Canyon Hard Fork] Deploy create2deployer Precompile