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

AcctIdx: cleanup bg threads #20731

Merged
merged 1 commit into from
Oct 15, 2021
Merged

Conversation

jeffwashington
Copy link
Contributor

@jeffwashington jeffwashington commented Oct 15, 2021

Problem

Refactoring, cleanup to make bg threads and lifetimes more clear.
Also got rid of threads on BucketMapHolder

Summary of Changes

Fixes #

@jeffwashington jeffwashington marked this pull request as ready for review October 15, 2021 16:40
brooksprumo
brooksprumo previously approved these changes Oct 15, 2021
Copy link
Contributor

@brooksprumo brooksprumo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me. No need for a re-review if you want to change _bg_threads or not.

Comment on lines 14 to 15
// bg_threads is 'never used'. It is only used to manage the lifetime
#[allow(dead_code)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of the #[allow(dead_code)], would an underscore on bg_threads work too? (i.e. _bg_threads)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good idea. done.

@mergify mergify bot dismissed brooksprumo’s stale review October 15, 2021 18:22

Pull request has been modified.

@codecov
Copy link

codecov bot commented Oct 15, 2021

Codecov Report

Merging #20731 (f28f636) into master (4ec65b6) will decrease coverage by 0.0%.
The diff coverage is 84.0%.

@@            Coverage Diff            @@
##           master   #20731     +/-   ##
=========================================
- Coverage    81.9%    81.8%   -0.1%     
=========================================
  Files         495      495             
  Lines      138041   138092     +51     
=========================================
- Hits       113072   113060     -12     
- Misses      24969    25032     +63     

@jeffwashington jeffwashington merged commit 47de4f3 into solana-labs:master Oct 15, 2021
jeffwashington added a commit to jeffwashington/solana that referenced this pull request Oct 25, 2021
jeffwashington added a commit to jeffwashington/solana that referenced this pull request Oct 25, 2021
jeffwashington added a commit to jeffwashington/solana that referenced this pull request Oct 26, 2021
jeffwashington added a commit to jeffwashington/solana that referenced this pull request Oct 26, 2021
jeffwashington added a commit that referenced this pull request Oct 26, 2021
* Revert part of "AcctIdx: cleanup bg threads (#20731)"

This reverts part of commit 47de4f3.

* change bg waiting metrics to percent
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
dankelleher pushed a commit to identity-com/solana that referenced this pull request Nov 24, 2021
* Revert part of "AcctIdx: cleanup bg threads (solana-labs#20731)"

This reverts part of commit 47de4f3.

* change bg waiting metrics to percent
frits-metalogix added a commit to identity-com/solana that referenced this pull request Nov 24, 2021
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

Successfully merging this pull request may close these issues.

2 participants