Skip to content

Fix Race in AsyncTwoPhaseIndexerTests.testStateMachine #40947

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

Merged
merged 1 commit into from
Apr 8, 2019

Conversation

original-brownbear
Copy link
Contributor

@original-brownbear original-brownbear commented Apr 8, 2019

  • The step is incremented by the listner in org.elasticsearch.xpack.core.indexing.AsyncTwoPhaseIndexerTests.MockIndexer#onFinish after isFinished is set to true, but the test only waited for isFinished,
    fixed by calling isFinished last
  • Also made step volatile since we are reading it from different thread from the one incrementing it
  • Closes AsyncTwoPhaseIndexerTests testStateMachine CI failures #40946

randomly ran into this and fixed it real quick

* The step is incremented by the listner in `org.elasticsearch.xpack.core.indexing.AsyncTwoPhaseIndexerTests.MockIndexer#onFinish` after isFinished is set to true, but the test only waited for `isFinished`,
fixed by calling `isFinished` last
* Also made `step` volatile since we are reading it from different thread from the one incrementing it
* Closes elastic#40946
@original-brownbear original-brownbear added >test Issues or PRs that are addressing/adding tests :Analytics/Geo Indexing, search aggregations of geo points and shapes v8.0.0 v7.2.0 :ml/Transform Transform labels Apr 8, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-analytics-geo

@elasticmachine
Copy link
Collaborator

Pinging @elastic/ml-core

Copy link

@hendrikmuhs hendrikmuhs left a comment

Choose a reason for hiding this comment

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

LGTM

@original-brownbear
Copy link
Contributor Author

thanks @hendrikmuhs !

@original-brownbear original-brownbear merged commit 00a44b0 into elastic:master Apr 8, 2019
@original-brownbear original-brownbear deleted the 37695 branch April 8, 2019 12:49
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Apr 8, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Apr 8, 2019
…e-unsafe-publication

* elastic/master:
  Update contributing docs to reflect JDK 11 (elastic#40955)
  Docs: Simplifying setup by using module configuration variant syntax (elastic#40879)
  Unmute CreateIndexIT testCreateAndDeleteIndexConcurrently CI failures (elastic#40960)
  Revert "Short-circuit rebalancing when disabled (elastic#40942)"
  Mute EnableAllocationShortCircuitTests
  SQL: Refactor args verification of In & conditionals (elastic#40916)
  Avoid sharing source directories as it breaks intellij (elastic#40877)
  Short-circuit rebalancing when disabled (elastic#40942)
  SQL: Prefer resultSets over exceptions in metadata (elastic#40641)
  Mute ClusterPrivilegeTests.testThatSnapshotAndRestore
  Fix Race in AsyncTwoPhaseIndexerTests.testStateMachine (elastic#40947)
  Relax Overly Strict Assertion in TransportShardBulkAction (elastic#40940)
original-brownbear added a commit to original-brownbear/elasticsearch that referenced this pull request Apr 9, 2019
* The step is incremented by the listner in `org.elasticsearch.xpack.core.indexing.AsyncTwoPhaseIndexerTests.MockIndexer#onFinish` after isFinished is set to true, but the test only waited for `isFinished`,
fixed by calling `isFinished` last
* Also made `step` volatile since we are reading it from different thread from the one incrementing it
* Closes elastic#40946
original-brownbear added a commit that referenced this pull request Apr 9, 2019
* The step is incremented by the listner in `org.elasticsearch.xpack.core.indexing.AsyncTwoPhaseIndexerTests.MockIndexer#onFinish` after isFinished is set to true, but the test only waited for `isFinished`,
fixed by calling `isFinished` last
* Also made `step` volatile since we are reading it from different thread from the one incrementing it
* Closes #40946
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
* The step is incremented by the listner in `org.elasticsearch.xpack.core.indexing.AsyncTwoPhaseIndexerTests.MockIndexer#onFinish` after isFinished is set to true, but the test only waited for `isFinished`,
fixed by calling `isFinished` last
* Also made `step` volatile since we are reading it from different thread from the one incrementing it
* Closes elastic#40946
gurkankaymak pushed a commit to gurkankaymak/elasticsearch that referenced this pull request May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Analytics/Geo Indexing, search aggregations of geo points and shapes :ml/Transform Transform >test Issues or PRs that are addressing/adding tests v7.2.0 v8.0.0-alpha1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

AsyncTwoPhaseIndexerTests testStateMachine CI failures
4 participants