-
Notifications
You must be signed in to change notification settings - Fork 20.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
core, ethdb, trie: mode dirty data to clean cache on flush #19307
Conversation
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.
LGTM
trie/database.go
Outdated
db.lock.Unlock() | ||
batch.Reset() | ||
} | ||
db.lock.RLock() |
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.
Yes, this is a cluster-f*ck. That said, the trie database write access is serialized by the blockchain importer, since we can't import two blocks concurrently. The only thing this code needs to ensure is that API CALL operations, sync state retrievals, etc don't mess up. The blockchain guarantees that no write will intervene here.
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 per our long discussion on Discord, please remove read locking from Commit
and Cap
. Please also document that Commit
and Cap
are not safe for concurrent use.
…19307) This PR is a more advanced form of the dirty-to-clean cacher (ethereum#18995), where we reuse previous database write batches as datasets to uncache, saving a dirty-trie-iteration and a dirty-trie-rlp-reencoding per block.
This PR is a more advanced form of the dirty-to-clean cacher (#18995), where we reuse previous database write batches as datasets to uncache, saving a dirty-trie-iteration and a dirty-trie-rlp-reencoding per block.