-
Notifications
You must be signed in to change notification settings - Fork 585
transition cache leak during ledger catchup #2168
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
transition cache leak during ledger catchup #2168
Conversation
…ent the result of processing transitions
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…aProtocol/coda into fix/ledger-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
…-catchup-be-aware-of-cache
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.
The PR looks good. Just have a question about test_ledger_catchup.ml
Also, is it possible to dedup the tests you wrote in test_ledger_catchup
?
src/lib/transition_frontier_controller_tests/test_ledger_catchup.ml
Outdated
Show resolved
Hide resolved
Also, after I approve this PR, can you update the documentation for |
I'm trying to understand the whole |
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. Just had some minor nits, mostly for readability.
I already addressed those in my PR
unprocessed_transition_cache
.ledger_catchup
would be blocked by transitions in cache, and the catchup would fail. This is super inefficient.This PR try to fix the above problems.