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

Use epoch as the gossip purge timeout for staked nodes. #7005

Merged
merged 4 commits into from
Nov 20, 2019

Conversation

aeyakovenko
Copy link
Member

@aeyakovenko aeyakovenko commented Nov 17, 2019

Problem

Gossip purge timeout is really short and will cause staked nodes to be purged during a relatively short partition.

Summary of Changes

Set the gossip timeout for staked nodes to be 1 epoch long.

Fixes #

tag: @sagar-solana

@aeyakovenko aeyakovenko changed the title Gossip epoch timeout for staked nodes. Use epoch as the gossip purge timeout for staked nodes. Nov 17, 2019
@aeyakovenko aeyakovenko marked this pull request as ready for review November 17, 2019 16:45
sagar-solana
sagar-solana previously approved these changes Nov 17, 2019
Copy link
Contributor

@sagar-solana sagar-solana left a comment

Choose a reason for hiding this comment

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

Minor comments.

@mergify mergify bot dismissed sagar-solana’s stale review November 17, 2019 23:03

Pull request has been modified.

@codecov
Copy link

codecov bot commented Nov 18, 2019

Codecov Report

Merging #7005 into master will decrease coverage by 7.7%.
The diff coverage is 57.5%.

@@           Coverage Diff            @@
##           master   #7005     +/-   ##
========================================
- Coverage      79%   71.2%   -7.8%     
========================================
  Files         225     227      +2     
  Lines       43369   48359   +4990     
========================================
+ Hits        34279   34460    +181     
- Misses       9090   13899   +4809

@aeyakovenko aeyakovenko force-pushed the gossip_wait branch 3 times, most recently from b463e02 to e670bf6 Compare November 19, 2019 17:33
@aeyakovenko aeyakovenko requested a review from carllin November 19, 2019 17:33
aeyakovenko and others added 4 commits November 20, 2019 10:37
* jemalloc heap memory tracker
* gossip recycler fixes
* partition test cleanup
@aeyakovenko aeyakovenko added the automerge Merge this Pull Request automatically once CI passes label Nov 20, 2019
@solana-grimes solana-grimes merged commit b150da8 into solana-labs:master Nov 20, 2019
aeyakovenko added a commit to aeyakovenko/solana that referenced this pull request Nov 20, 2019
@@ -1385,6 +1385,7 @@ impl Bank {
/// A snapshot bank should be purged of 0 lamport accounts which are not part of the hash
/// calculation and could shield other real accounts.
pub fn verify_snapshot_bank(&self) -> bool {
self.purge_zero_lamport_accounts();
Copy link
Member

@ryoqun ryoqun Nov 21, 2019

Choose a reason for hiding this comment

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

@aeyakovenko Err, I think this should be removed because this is cherry-picked from #7010, which is not a correct fix (I'm working on it), and isn't related to this PR.

Copy link
Member

Choose a reason for hiding this comment

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

back ref: #7010 (comment)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
automerge Merge this Pull Request automatically once CI passes
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants