-
Notifications
You must be signed in to change notification settings - Fork 15
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
Fix get_all_balances returning zero objects count in fullnode #4828
base: develop
Are you sure you want to change the base?
Fix get_all_balances returning zero objects count in fullnode #4828
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
5974337
to
4ba1c82
Compare
@@ -1367,6 +1367,15 @@ impl IndexStore { | |||
}) | |||
}) | |||
.await | |||
.map(|balances_map| { |
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.
I'm not sure I understand this part to 100%, but wouldn't that mean we never cache entries with 0 balance, and therefore with every lookup we have expensive DB lookups now?
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 filtering is done at the very end, so it's not affecting the process of reading from and filling the cache.
We are storing the 0 balance entries in cache just like before.
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 🎈
.map(|balances_map| { | ||
Arc::new( | ||
(*balances_map) | ||
.clone() | ||
.into_iter() | ||
.filter(|(_, TotalBalance { num_coins, .. })| *num_coins > 0) | ||
.collect::<HashMap<_, _>>(), | ||
) |
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.
Nit: It would save the reallocation, and reconstruction of the map
.map(|balances_map| { | |
Arc::new( | |
(*balances_map) | |
.clone() | |
.into_iter() | |
.filter(|(_, TotalBalance { num_coins, .. })| *num_coins > 0) | |
.collect::<HashMap<_, _>>(), | |
) | |
.map(|mut balances_map| { | |
Arc::make_mut(&mut balances_map) | |
.retain(|_, TotalBalance { num_coins, .. }| *num_coins > 0); | |
balances_map |
Description of change
Implement filtering of balances returned by
get_all_balances
to omit coins with zero objects count.The most probable root cause of the issue was described here: #4349 (comment)
Links to any relevant issues
#4349
Type of change
How the change has been tested
cargo test --package=iota-indexer --profile=simulator --features shared_test_runtime
cargo nextest run --package=iota-json-rpc-tests
Change checklist
Tick the boxes that are relevant to your changes, and delete any items that are not.