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

[CI] SmokeTestWatcherWithSecurityIT.testSearchInputHasPermissions flaky failures #33320

Closed
talevy opened this issue Aug 31, 2018 · 6 comments
Closed
Assignees
Labels
:Data Management/Watcher >test-failure Triaged test failures from CI

Comments

@talevy
Copy link
Contributor

talevy commented Aug 31, 2018

master and 6.x have been running into many instances of this test failing

here is one reproduce snippet that doesn't always reproduce

REPRODUCE WITH: ./gradlew :x-pack:qa:smoke-test-watcher-with-security:integTestRunner \
  -Dtests.seed=7EF35AACC2EC51CB \
  -Dtests.class=org.elasticsearch.smoketest.SmokeTestWatcherWithSecurityIT \
  -Dtests.method="testSearchInputHasPermissions" \
  -Dtests.security.manager=true \
  -Dtests.locale=sr-Latn-BA \
  -Dtests.timezone=Pacific/Fakaofo \
  -Dcompiler.java=10 \
  -Druntime.java=10
@talevy talevy added >test-failure Triaged test failures from CI :Data Management/Watcher labels Aug 31, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

talevy added a commit that referenced this issue Aug 31, 2018
talevy added a commit that referenced this issue Aug 31, 2018
@Tim-Brooks
Copy link
Contributor

Thanks for doing this. I was just about to mute these tests myself.

Tim-Brooks added a commit that referenced this issue Aug 31, 2018
In the previous commit where SmokeTestWatcherWithSecurityIT tests were
muted, I added the incorrect issue numbers. This commit fixes this. The
issue for the tests is #33320.
Tim-Brooks added a commit that referenced this issue Aug 31, 2018
In the previous commit where SmokeTestWatcherWithSecurityIT tests were
muted, I added the incorrect issue numbers. This commit fixes this. The
issue for the tests is #33320.
@Tim-Brooks
Copy link
Contributor

I also muted testSearchTransformHasPermissions and testIndexActionInsufficientPrivileges because they also appear to be failing.

@talevy
Copy link
Contributor Author

talevy commented Aug 31, 2018

thanks @tbrooks8! I kept bouncing back and forth between how many to mute. For whatever reason, they are all going to town recently

@spinscale spinscale self-assigned this Sep 3, 2018
spinscale added a commit to spinscale/elasticsearch that referenced this issue Sep 3, 2018
This commit reverts most of elastic#33157 as it introduces another race
condition and breaks a common case of watcher, when the first watch is
added to the system and the index does not exist yet.

This means, that the index will be created, which triggers a reload, but
during this time the put watch operation that triggered this is not yet
indexed, so that both processes finish roughly add the same time and
should not overwrite each other but act complementary.

This commit reverts the logic of cleaning out the ticker engine watches
on start up, as this is done already when the execution is paused -
which also gets paused on the cluster state listener again, as we can be
sure here, that the watches index has not yet been created.

This also adds a new test, that starts a one node cluster and emulates
the case of a non existing watches index and a watch being added, which
should result in proper execution.

Closes elastic#33320
spinscale added a commit that referenced this issue Sep 21, 2018
…3360)

This commit reverts most of #33157 as it introduces another race
condition and breaks a common case of watcher, when the first watch is
added to the system and the index does not exist yet.

This means, that the index will be created, which triggers a reload, but
during this time the put watch operation that triggered this is not yet
indexed, so that both processes finish roughly add the same time and
should not overwrite each other but act complementary.

This commit reverts the logic of cleaning out the ticker engine watches
on start up, as this is done already when the execution is paused -
which also gets paused on the cluster state listener again, as we can be
sure here, that the watches index has not yet been created.

This also adds a new test, that starts a one node cluster and emulates
the case of a non existing watches index and a watch being added, which
should result in proper execution.

Closes #33320
spinscale added a commit that referenced this issue Sep 21, 2018
…3360)

This commit reverts most of #33157 as it introduces another race
condition and breaks a common case of watcher, when the first watch is
added to the system and the index does not exist yet.

This means, that the index will be created, which triggers a reload, but
during this time the put watch operation that triggered this is not yet
indexed, so that both processes finish roughly add the same time and
should not overwrite each other but act complementary.

This commit reverts the logic of cleaning out the ticker engine watches
on start up, as this is done already when the execution is paused -
which also gets paused on the cluster state listener again, as we can be
sure here, that the watches index has not yet been created.

This also adds a new test, that starts a one node cluster and emulates
the case of a non existing watches index and a watch being added, which
should result in proper execution.

Closes #33320
kcm pushed a commit that referenced this issue Oct 30, 2018
…3360)

This commit reverts most of #33157 as it introduces another race
condition and breaks a common case of watcher, when the first watch is
added to the system and the index does not exist yet.

This means, that the index will be created, which triggers a reload, but
during this time the put watch operation that triggered this is not yet
indexed, so that both processes finish roughly add the same time and
should not overwrite each other but act complementary.

This commit reverts the logic of cleaning out the ticker engine watches
on start up, as this is done already when the execution is paused -
which also gets paused on the cluster state listener again, as we can be
sure here, that the watches index has not yet been created.

This also adds a new test, that starts a one node cluster and emulates
the case of a non existing watches index and a watch being added, which
should result in proper execution.

Closes #33320
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Data Management/Watcher >test-failure Triaged test failures from CI
Projects
None yet
Development

No branches or pull requests

4 participants