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

Fixup flaky tests #21617

Merged
merged 2 commits into from
Dec 6, 2021
Merged

Fixup flaky tests #21617

merged 2 commits into from
Dec 6, 2021

Conversation

carllin
Copy link
Contributor

@carllin carllin commented Dec 5, 2021

Problem

do_test_optimistic_confirmation_violation_with_or_without_tower() local cluster tests are flaky for a few reasons:

  1. If the cluster immediately forks on restart while we're killing validators A and C, with Validator B on one side, and A and C on a heavier fork, it's possible that the lockouts on A and C's latest votes do not extend past validator B's latest vote. Then validator B will be stuck unable to vote, but also unable generate a switching proof to the heavier fork.

  2. Validator A can vote past next_slot_on_a before we can kill it at the beginning of the test. When we then copy over validator B's ledger below only for slots <= next_slot_on_a, then validator A loses the slot it last voted on next_slot_on_a from its ledger, even though it retains it in its tower. Thus it will not know how its last vote chains to the base_slot, and will thus count votes for ancestors of its last vote towards the switching proof, and thus may violate switching proofs on restart.

Summary of Changes

Fixes #

@CriesofCarrots
Copy link
Contributor

🙏

@codecov
Copy link

codecov bot commented Dec 6, 2021

Codecov Report

Merging #21617 (2230b2e) into master (cd17f63) will decrease coverage by 0.0%.
The diff coverage is n/a.

@@            Coverage Diff            @@
##           master   #21617     +/-   ##
=========================================
- Coverage    81.6%    81.6%   -0.1%     
=========================================
  Files         510      510             
  Lines      142504   142504             
=========================================
- Hits       116401   116388     -13     
- Misses      26103    26116     +13     

@carllin carllin force-pushed the FixLocalClusterTests branch from 2b393aa to 9eec5a9 Compare December 6, 2021 05:02
@carllin carllin force-pushed the FixLocalClusterTests branch from 9eec5a9 to 2230b2e Compare December 6, 2021 18:30
@carllin carllin marked this pull request as ready for review December 6, 2021 21:26
@carllin carllin requested a review from AshwinSekar December 6, 2021 21:26
@carllin carllin merged commit f493a88 into solana-labs:master Dec 6, 2021
mergify bot pushed a commit that referenced this pull request Dec 6, 2021
* Fixup flaky tests

* Fixup listeners

(cherry picked from commit f493a88)

# Conflicts:
#	local-cluster/tests/local_cluster.rs
mergify bot pushed a commit that referenced this pull request Dec 6, 2021
* Fixup flaky tests

* Fixup listeners

(cherry picked from commit f493a88)
carllin added a commit that referenced this pull request Dec 7, 2021
* Fixup flaky tests

* Fixup listeners
mergify bot added a commit that referenced this pull request Dec 7, 2021
* Fixup flaky tests

* Fixup listeners

(cherry picked from commit f493a88)

Co-authored-by: carllin <[email protected]>
mergify bot added a commit that referenced this pull request Dec 7, 2021
* Fix fork race condition in optimistic violation tower tests (#19192)

* Fix fork race condition in optimistic violation tower tests

* clippy

* pr comments

* Fixup flaky tests (#21617)

* Fixup flaky tests

* Fixup listeners

Co-authored-by: Ashwin Sekar <[email protected]>
Co-authored-by: carllin <[email protected]>
t-nelson added a commit to t-nelson/solana that referenced this pull request Dec 13, 2021
tao-stones pushed a commit that referenced this pull request Dec 13, 2021
CriesofCarrots pushed a commit to CriesofCarrots/solana that referenced this pull request Dec 16, 2021
CriesofCarrots pushed a commit that referenced this pull request Dec 17, 2021
@brooksprumo brooksprumo mentioned this pull request Jan 5, 2022
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