Skip to content

Wait for blackholed connection before discovery #44077

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

Conversation

DaveCTurner
Copy link
Contributor

Since #42636 we no longer treat connections specially when simulating a
blackholed connection. This means that at the end of the safety phase we may
have just started a connection attempt which will time out, but the default
timeout is 30 seconds, much longer than the 2 seconds we normally allow for
post-safety-phase discovery. This commit adds time for such a connection
attempt to time out.

It also fixes some spurious logging of this that now refers to an object with
an unhelpful toString() implementation introduced in #42636.

Fixes #44073

Since elastic#42636 we no longer treat connections specially when simulating a
blackholed connection. This means that at the end of the safety phase we may
have just started a connection attempt which will time out, but the default
timeout is 30 seconds, much longer than the 2 seconds we normally allow for
post-safety-phase discovery. This commit adds time for such a connection
attempt to time out.

It also fixes some spurious logging of `this` that now refers to an object with
an unhelpful `toString()` implementation introduced in elastic#42636.

Fixes elastic#44073
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v8.0.0 v7.4.0 labels Jul 8, 2019
@DaveCTurner DaveCTurner requested review from andrershov and ywelsch July 8, 2019 15:11
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks

@DaveCTurner
Copy link
Contributor Author

@elasticmachine please run elasticsearch-ci/packaging-sample

@DaveCTurner
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1 run elasticsearch-ci/2 run elasticsearch-ci/bwc run elasticsearch-ci/default-distro run elasticsearch-ci/docbldesx run elasticsearch-ci/oss-distro-docs

@DaveCTurner DaveCTurner merged commit bf2f103 into elastic:master Jul 9, 2019
@DaveCTurner DaveCTurner deleted the 2019-07-08-more-time-for-discovery-44073 branch July 9, 2019 09:59
DaveCTurner added a commit that referenced this pull request Jul 9, 2019
Since #42636 we no longer treat connections specially when simulating a
blackholed connection. This means that at the end of the safety phase we may
have just started a connection attempt which will time out, but the default
timeout is 30 seconds, much longer than the 2 seconds we normally allow for
post-safety-phase discovery. This commit adds time for such a connection
attempt to time out.

It also fixes some spurious logging of `this` that now refers to an object with
an unhelpful `toString()` implementation introduced in #42636.

Fixes #44073
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Coordination/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. >test Issues or PRs that are addressing/adding tests v7.4.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoordinatorTests testAckListenerReceivesNacksIfPublicationTimesOut failure
4 participants