-
Notifications
You must be signed in to change notification settings - Fork 550
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
Handle non-existent accounts in preload #15978
Handle non-existent accounts in preload #15978
Conversation
Problem: account preloading is not able to preserve information about some accounts not existing inledger, which invites future repetetive lookups to DB during staged ledger application. Solution: keep a set of non-existent accounts for masks populated on preload.
d0f34ae
to
99ed169
Compare
if List.is_empty not_found then | ||
List.map ~f:(fun (a, x) -> (a, Option.value_exn x)) self_found_or_none | ||
else | ||
let from_parent = lookup_parent ancestor not_found in |
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.
This code refactor is purely for clarity that call to parent happens only if some records were not found "locally" and underlying ledger needs to be queried.
Ensures no hash db lookups for non-existent accounts even in edge cases.
* use `t.current_location` instead of `last_filled t` * check against `non_existent_accounts` in `get_or_create_account` Ensure that underlying ledger won't be unnecessarily queried using the `last_filled`.
!ci-build-me |
…-non-existent-accounts-in-preload
!ci-build-me |
A unit test is failing because of reparenting logic. This is a unit test for dead code effectively. I.e. the code is used, but never in a way suggested by the test. Still some code change is needed to either fix the dead code or remove it. |
The test is junk, feel free to just delete it. |
!ci-build-me |
So the actual issue was not in reparenting logic, but in the fact that registering a mask happened before the last edit of the parent. This is something we don't do in production code, but were doing in the test and with the last commit of this PR it started failing the test because instead of calling the In near future I hope we rewrite the masking code so that it doesn't allow modification of a mask that has some children For now I fixed the test to make mask-attaching happen after edits to the base mask performed. |
Problem: account preloading is not able to preserve information about some accounts not existing in ledger, which invites future repetitive lookups to DB during staged ledger application.
Solution: keep a set of non-existent accounts for masks populated on preload.
Impact: after preload underlying ledger is guaranteed not to be queried.
Explain how you tested your changes:
Checklist: