fix: ethhashlookup: clean-up query management and lifecycle #12235
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rvagg
commented
Jul 15, 2024
|
Running master along with #12236, my WAL is a reasonable size now. |
rvagg
force-pushed
the
rvagg/ethhashlookupdb-cleanup
branch
from
d43f6b4 to
acdc069
Compare
|
@rvagg Did this really reduce the size of your WAL ? How does that work ? IIUC, the only positive impact here should be reduced heap usage because of not leaking prepared statements anymore. |
|
@rvagg Just confirming that this was fixed and not rerun: |
Yes, fixed by changing the ordering of |
rvagg
force-pushed
the
rvagg/ethhashlookupdb-cleanup
branch
from
acdc069 to
66a0ccc
Compare
aarshkshah1992
approved these changes
Jul 16, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #12171
Prompted because I'm noticing that my WAL for both calibnet and mainnet nodes is ~32M, even with my calibnet node running master which has #12098 which should result in it being compacted earlier than that (suggests that queries may be remaining open?).
The main thing that stands out in this file is the use of the preparedstatement that's prepared each time it's used, and not closed. So that's what's probably retaining memory and perhaps it's also responsible for the WAL size (I'm not sure how though tbh).
This refactor makes all queries use preparedstatements that persist for the lifecycle of the index, so we get the efficiency gains from that.