Skip to content

Rework AckIT tests #33673

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.

Sign up for GitHub

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

Jump to bottom
Closed
ywelsch opened this issue Sep 13, 2018 · 5 comments
Closed

Rework AckIT tests #33673

ywelsch opened this issue Sep 13, 2018 · 5 comments
Labels
:Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. help wanted adoptme

Comments

@ywelsch
Copy link
Contributor

ywelsch commented Sep 13, 2018

#30672 fixed the acking mechanism in Elasticsearch, which would previously just erroneously report failures as successful acks. The AckIT tests that accompanied the acking implementation are unfortunately fundamentally broken and would have never worked if the acking implementation had not been broken since its inception. The reason that the acking cannot work in this class is that it uses a publish timeout of 0, which can result in cluster states to arrive out-of-order on nodes, which can in certain situations lead to cluster states being rejected as a better cluster state was already fully applied on the node. One way to work around the limitations in the test here might be to bring the cluster to a stable state first, and only then change the publish timeout for the one publishing round we want to check the acking for.

The AckIT tests have been disabled as part of #32767 and should be reworked / possibly replaced by a new set of tests.
@ywelsch ywelsch added help wanted adoptme :Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. labels Sep 13, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed

@ywelsch ywelsch mentioned this issue Sep 13, 2018
Closed
@ywelsch ywelsch mentioned this issue Sep 25, 2018
Closed
imotov added a commit that referenced this issue Sep 25, 2018
@imotov
Mute AckClusterUpdateSettingsIT
1e6780d 
Tracked by #33673
@imotov
Copy link
Contributor

imotov commented Sep 25, 2018

As per discussion with @ywelsch in #34053 I have disabled AckClusterUpdateSettingsIT in master as well. I am going to close #34053 in favor of this issue.

@DaveCTurner
Copy link
Contributor

See also https://elasticsearch-ci.elastic.co/job/elastic+elasticsearch+master+multijob-unix-compatibility/os=opensuse/58/console

kcm pushed a commit that referenced this issue Oct 30, 2018
@imotov @kcm
Mute AckClusterUpdateSettingsIT
423497c 
Tracked by #33673
@tvernum tvernum mentioned this issue Dec 19, 2018
Closed
@tlrx tlrx mentioned this issue Feb 1, 2019
Closed
@tlrx
Copy link
Member

tlrx commented Feb 1, 2019

I have disabled AckClusterUpdateSettingsIT in 6.x and closed #37478 in favor of this issue.

tlrx added a commit that referenced this issue Feb 1, 2019
@tlrx
Disable all tests in AckClusterUpdateSettingsIT
d0d0c89 
Those tests are broken like explained in #33673. Similar tests have
already been muted (see #32767) and this specific class has been muted
in master already (see #34053).

Closes #37478
@ywelsch
Copy link
Contributor Author

ywelsch commented Mar 12, 2019

These tests have been removed and replaced with others as part of the Zen2 work.

@ywelsch ywelsch closed this as completed Mar 12, 2019
cbuescher pushed a commit to cbuescher/elasticsearch that referenced this issue Jul 31, 2019
Remove muted AckIT and AckClusterUpdateSettingsIT
3f78afd 
Reading up on elastic#33673 it looks like parts of these tests have been reworked and
there is no intention to fix the remains on 7.x, so I think we can remove the
entire test.
@cbuescher cbuescher mentioned this issue Jul 31, 2019
Merged
cbuescher pushed a commit that referenced this issue Jul 31, 2019
Remove muted AckIT and AckClusterUpdateSettingsIT (#45044)
35291ae 
Reading up on #33673 it looks like parts of these tests have been reworked and
there is no intention to fix the remains on 7.x, so I think we can remove the
entire test.
cbuescher pushed a commit that referenced this issue Jul 31, 2019
Remove muted AckIT and AckClusterUpdateSettingsIT (#45044)
6208b2b 
Reading up on #33673 it looks like parts of these tests have been reworked and
there is no intention to fix the remains on 7.x, so I think we can remove the
entire test.
cbuescher pushed a commit that referenced this issue Jul 31, 2019
Remove muted AckIT and AckClusterUpdateSettingsIT (#45044)
d3b1091 
Reading up on #33673 it looks like parts of these tests have been reworked and
there is no intention to fix the remains on 7.x, so I think we can remove the
entire test.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
:Distributed Indexing/Distributed A catch all label for anything in the Distributed Indexing Area. Please avoid if you can. help wanted adoptme
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
5 participants
@tlrx @imotov @ywelsch @DaveCTurner @elasticmachine