-
Notifications
You must be signed in to change notification settings - Fork 958
Make the block cache optional #8066
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
Conversation
|
Fixed the tests, there was just a small issue with us not deleting from the state cache when the block cache was disabled. The fix is to always delete from the state cache, I don't think this should impose too much overhead 🙏 But re-testing with a backfilling supernode should confirm this. |
jimmygchen
left a comment
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.
Looks good!
Tested this earlier and this fixes the slow persist_block_and_blobs and rpc serving behaviour 🎉
Proposed Changes
Address contention on the store's
block_cacheby allowing it to be disabled when--block-cache-size 0is provided, and also making this the default.Additional Info
It would also be good to add a metric on
block_cache.lock(), but I think this would require abstracting/copying some of theCanonicalHeadRwLockstuff and I haven't done that yet.