Skip to content

Fix ClusterDisruptionIT.testAckedIndexing #53169

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

henningandersen
Copy link
Contributor

Use assertBusy when doing reroute after bridged disruption,
since it can return non-acked if a node is marked faulty
by follower check after disruption ended.

Closes #53064

Use assertBusy when doing reroute after bridged disruption,
since it can return non-acked if a node is marked faulty
by follower check after disruption ended.

Closes elastic#53064
@henningandersen henningandersen added >test Issues or PRs that are addressing/adding tests :Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. v8.0.0 v7.7.0 v7.6.2 labels Mar 5, 2020
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (:Distributed/CRUD)

Copy link
Member

@dnhatn dnhatn 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 Henning.

@henningandersen
Copy link
Contributor Author

Thanks for reviewing, Nhat.

@henningandersen henningandersen merged commit 97c6174 into elastic:master Mar 6, 2020
henningandersen added a commit that referenced this pull request Mar 6, 2020
Use assertBusy when doing reroute after bridged disruption,
since it can return non-acked if a node is marked faulty
by follower check after disruption ended.

Closes #53064
henningandersen added a commit that referenced this pull request Mar 6, 2020
Use assertBusy when doing reroute after bridged disruption,
since it can return non-acked if a node is marked faulty
by follower check after disruption ended.

Closes #53064
@romseygeek
Copy link
Contributor

This test has also failed in 6.8, see https://gradle-enterprise.elastic.co/s/t6xoiqr4ngnp4

Should we backport the fix there as well?

@henningandersen
Copy link
Contributor Author

@romseygeek , unfortunately that test failure is in a different place:

   > Expected: <true>
   >      but: was <false>
   >    at __randomizedtesting.SeedInfo.seed([74F3D491E0E2587B:FE326062BC9FBE30]:0)
   >    at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
   >    at org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked(ElasticsearchAssertions.java:109)
   >    at org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked(ElasticsearchAssertions.java:113)
   >    at org.elasticsearch.test.TestCluster.wipeIndices(TestCluster.java:142)
   >    at org.elasticsearch.test.TestCluster.wipe(TestCluster.java:79)
   >    at org.elasticsearch.test.ESIntegTestCase.afterInternal(ESIntegTestCase.java:592)
   >    at org.elasticsearch.test.ESIntegTestCase.cleanUpCluster(ESIntegTestCase.java:2205)
   >    at java.lang.Thread.run(Thread.java:748)

I find it reasonable to ignore this one incident since so much has happened in 7.x w.r.t. cluster healing (new coordination layer for instance) that it hardly makes sense to focus on this in 6.x. I would opt to leave the test unmuted in 6.x, unless this reoccurs too frequently (in case it fails in other interesting ways).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed Indexing/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >test Issues or PRs that are addressing/adding tests v7.6.2 v7.7.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ClusterDisruptionIT.testAckedIndexing failure
5 participants