Skip to content
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

coverage test is flaky recently #27570

Closed
yihau opened this issue Sep 2, 2022 · 3 comments
Closed

coverage test is flaky recently #27570

yihau opened this issue Sep 2, 2022 · 3 comments

Comments

@yihau
Copy link
Member

yihau commented Sep 2, 2022

Problem

I can reproduce the error by cargo test -p solana-runtime and it is intermittent. would like to share some investigation here.

https://github.com/solana-labs/solana/blob/master/runtime/src/prioritization_fee_cache.rs#L482-L487

            let fee = PrioritizationFeeCache::get_prioritization_fee(
                prioritization_fee_cache.cache.clone(),
                &slot,
            );
            let fee = fee.lock().unwrap();
            assert_eq!(2, fee.get_min_transaction_fee().unwrap()); <--- die here

If I'm correct, prioritization_fee_cache.cache is updated by a function called service_loop. The function lives in another thread. If we do get_prioritization_fee via prioritization_fee_cache.cache before service_loop, we will get a default object. When we try to call get_min_transaction_fee from the default object, I think we will get a None.

Proposed Solution

maybe ensure the cache has been updated. not sure is there any determine way to do it.

cc @taozhu-chicago @willhickey @CriesofCarrots

@yihau
Copy link
Member Author

yihau commented Sep 2, 2022

seems Tao has fixed it. #27572

@yihau yihau closed this as completed Sep 2, 2022
@tao-stones
Copy link
Contributor

seems Tao has fixed it. #27572

Thanks for confirming the issue. I thought I updated all tests to call sync_update() to ensure cache's updated before asserting. But apparently i missed this one.

@tao-stones
Copy link
Contributor

While looking into it, I also noticed that prioritization_fee_cache::drop() isn't necessary, if we are OK with not calling join() explicitly. As in #27568. wdyt @willhickey @CriesofCarrots

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants