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

increases timeout duration for gossip discover #17408

Merged
merged 2 commits into from
May 22, 2021

Conversation

behzadnouri
Copy link
Contributor

@behzadnouri behzadnouri commented May 22, 2021

Problem

#17236 increased default number of bloom items when making gossip
pull-requests. The change is effectively a no-op once a validator syncs
to a real cluster as the number of items will already become larger than
the default value there.

However, it adds overhead to tests with a very small cluster, and
results in dicover_cluster timeouts in tests.

Summary of Changes

  • 1st commit increases timeout in discover_cluster.
  • 2nd commit uses Duration type for timeout (type-safety and self-documenting unit).

@behzadnouri behzadnouri force-pushed the discover-fail branch 2 times, most recently from 0953aa5 to 9a3d61f Compare May 22, 2021 15:22
@codecov
Copy link

codecov bot commented May 22, 2021

Codecov Report

Merging #17408 (403f545) into master (a7870cd) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

@@            Coverage Diff            @@
##           master   #17408     +/-   ##
=========================================
- Coverage    82.7%    82.7%   -0.1%     
=========================================
  Files         424      424             
  Lines      118634   118634             
=========================================
- Hits        98216    98191     -25     
- Misses      20418    20443     +25     

@behzadnouri behzadnouri merged commit cf1acfb into solana-labs:master May 22, 2021
@behzadnouri behzadnouri deleted the discover-fail branch May 22, 2021 19:17
mergify bot added a commit that referenced this pull request May 22, 2021
)

* increases timeout duration for gossip discover

(cherry picked from commit d649637)

* uses Duration type for gossip discover timeout

(cherry picked from commit cf1acfb)

# Conflicts:
#	core/src/gossip_service.rs

* removes backport merge conflicts

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
behzadnouri added a commit to behzadnouri/solana that referenced this pull request May 23, 2021
coverage ci builds are failing presumably because of the overhead added
in: solana-labs#17236
for very small test cluster.

Previous attempt at fixing the flakiness:
solana-labs#17408

This commit uses smaller min number of bloom items condition on that if
debug assertions are enabled or not.
behzadnouri added a commit to behzadnouri/solana that referenced this pull request May 23, 2021
coverage ci builds are have become flaky presumably because of the
overhead added in solana-labs#17236
for very small test clusters.

This commit uses a smaller min number of bloom items condition on that
if debug assertions are enabled or not.

Previous attempt at fixing the flakiness:
solana-labs#17408
behzadnouri added a commit that referenced this pull request May 23, 2021
coverage ci builds are have become flaky presumably because of the
overhead added in #17236
for very small test clusters.

This commit uses a smaller min number of bloom items condition on that
if debug assertions are enabled or not.

Previous attempt at fixing the flakiness:
#17408
mergify bot pushed a commit that referenced this pull request May 23, 2021
coverage ci builds are have become flaky presumably because of the
overhead added in #17236
for very small test clusters.

This commit uses a smaller min number of bloom items condition on that
if debug assertions are enabled or not.

Previous attempt at fixing the flakiness:
#17408

(cherry picked from commit 5567305)
mergify bot added a commit that referenced this pull request May 23, 2021
coverage ci builds are have become flaky presumably because of the
overhead added in #17236
for very small test clusters.

This commit uses a smaller min number of bloom items condition on that
if debug assertions are enabled or not.

Previous attempt at fixing the flakiness:
#17408

(cherry picked from commit 5567305)

Co-authored-by: behzad nouri <behzadnouri@gmail.com>
@brooksprumo brooksprumo mentioned this pull request Aug 23, 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.

None yet

1 participant