-
Notifications
You must be signed in to change notification settings - Fork 592
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
Database stability improvements #1154
Conversation
Codecov Report
@@ Coverage Diff @@
## v7.x #1154 +/- ##
=======================================
Coverage 20.86% 20.86%
=======================================
Files 199 199
Lines 25416 25416
=======================================
Hits 5303 5303
Misses 19150 19150
Partials 963 963 Continue to review full report at Codecov.
|
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! Is the changing the hardcoded constant from 780k to 100k in this branch as well? (I didn't see it on skim in IAVL / cosmos SDK changes)
I think we should publish a release once thats in
@ValarDragon I'm working on that today to represent the limit in terms of bytes and as a configurable value. Hoping to be done by EOD and merge in a separate PR. If you'd like to get it out ASAP, I can hardcode it to 100K instead of the configurable bytes alternative I'm currently working on |
Oh sweet, didn't realize your going for the limit in bytes at the moment, I thought that was something we'd do much later in the future. I think we should change it to 100k and get that into a release (the release can still come out tomorrow!) I'm in favor of the limiting by bytes and a config value, but I don't think we should block the next release on that |
Sounds good. I will prepare the PRs with 100K right now. Will still keep working on the configurable value. If that doesn't get in before the next release, that's okay. |
My main thinking as to why, is because we may get unforeseen troubles that would delay a release to mid-week with the change to bytes. (E.g. some unexpected edge case, takes longer to review on time, etc.) I would rather get this out sooner, so we can get more information around the app hash situation, and mitigate the ongoing RAM usage concerns |
Yeah, we can still make a release as soon as the config gets done / merged! |
@ValarDragon updated sdk and iavl with smaller fast node cache size. Also, prepared changelog for |
Closes: #XXX
Description
For contributor use:
docs/
) or specification (x/<module>/spec/
)Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorer