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

Fix node health-check-related test failures #59277

Merged

Conversation

DaveCTurner
Copy link
Contributor

In #52680 we introduced a new health check mechanism. This commit fixes
up some sporadic related test failures, and improves the behaviour of
the FollowersChecker slightly in the case that no retries are
configured.

Closes #59252
Closes #59172

In elastic#52680 we introduced a new health check mechanism. This commit fixes
up some sporadic related test failures, and improves the behaviour of
the `FollowersChecker` slightly in the case that no retries are
configured.

Closes elastic#59252
Closes elastic#59172
@DaveCTurner DaveCTurner added >test Issues or PRs that are addressing/adding tests :Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. v8.0.0 v7.9.0 labels Jul 9, 2020
@DaveCTurner DaveCTurner requested a review from ywelsch July 9, 2020 06:41
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/Cluster Coordination)

@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team label Jul 9, 2020
@@ -327,16 +327,16 @@ public void handleException(TransportException exp) {
failureCountSinceLastSuccess++;

final String reason;
if (failureCountSinceLastSuccess >= followerCheckRetryCount) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We should check the failure count last so we can report more specific errors (disconnected etc) if available.

@@ -325,7 +324,7 @@ public FileChannel newFileChannel(Path path, Set<? extends OpenOption> options,
@Override
public void force(boolean metaData) throws IOException {
if (injectIOException.get()) {
if (path.toString().startsWith(pathPrefix) && path.toString().endsWith(".es_temp_file")) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

No need to check the path starts with / and, indeed, it doesn't on Windows.

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

@DaveCTurner DaveCTurner merged commit 5e73d71 into elastic:master Jul 9, 2020
@DaveCTurner DaveCTurner deleted the 2020-07-09-fix-followercheckertests branch July 9, 2020 11:46
DaveCTurner added a commit that referenced this pull request Jul 9, 2020
In #52680 we introduced a new health check mechanism. This commit fixes
up some sporadic related test failures, and improves the behaviour of
the `FollowersChecker` slightly in the case that no retries are
configured.

Closes #59252
Closes #59172
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/Cluster Coordination Cluster formation and cluster state publication, including cluster membership and fault detection. Team:Distributed Meta label for distributed team >test Issues or PRs that are addressing/adding tests v7.9.0 v8.0.0-alpha1
Projects
None yet
4 participants