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

AsyncTwoPhaseIndexerTests race condition fixed #37830

Merged
merged 4 commits into from
Jan 25, 2019

Conversation

pgomulka
Copy link
Contributor

@pgomulka pgomulka commented Jan 24, 2019

The unlucky timing can cause this test to fail when the indexing triggered from maybeTriggerAsyncJob. As this is asynchronous, in can finish quicker then the test stepping over to next assertion
The introduced barrier solves the problem
closes #37695

@pgomulka pgomulka self-assigned this Jan 24, 2019
@pgomulka pgomulka added :Core/Infra/Core Core issues without another label >test-failure Triaged test failures from CI labels Jan 24, 2019
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-core-infra

indexer.start();
assertThat(indexer.getState(), equalTo(IndexerState.STARTED));
assertTrue(indexer.maybeTriggerAsyncJob(System.currentTimeMillis()));
Thread.sleep(1000);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

to be removed

@pgomulka pgomulka requested a review from costin January 25, 2019 09:20
Copy link
Member

@costin costin left a comment

Choose a reason for hiding this comment

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

LGTM

assertThat(step, equalTo(3));
++step;
return new IterationResult<Integer>(Collections.emptyList(), 3, true);
}

private void awaitForLatch() {
try {
latch.await();
Copy link
Member

Choose a reason for hiding this comment

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

Please put a timeout (10s or whatever you think it's appropriate) to prevent this for hanging up.

@pgomulka
Copy link
Contributor Author

@elasticmachine run elasticsearch-ci/1

@pgomulka pgomulka merged commit 85acc11 into elastic:master Jan 25, 2019
jasontedor added a commit to jasontedor/elasticsearch that referenced this pull request Jan 25, 2019
* elastic/master: (68 commits)
  Fix potential IllegalCapacityException in LLRC when selecting nodes (elastic#37821)
  Mute CcrRepositoryIT#testFollowerMappingIsUpdated
  Fix S3 Repository ITs When Docker is not Available (elastic#37878)
  Pass distribution type through to docs tests (elastic#37885)
  Mute SharedClusterSnapshotRestoreIT#testSnapshotCanceledOnRemovedShard
  SQL: Fix casting from date to numeric type to use millis (elastic#37869)
  Migrate o.e.i.r.RecoveryState to Writeable (elastic#37380)
  ML: removing unnecessary upgrade code (elastic#37879)
  Relax cluster metadata version check (elastic#37834)
  Mute TransformIntegrationTests#testSearchTransform
  Refactored GeoHashGrid unit tests (elastic#37832)
  Fixes for a few randomized agg tests that fail hasValue() checks
  Geo: replace intermediate geo objects with libs/geo (elastic#37721)
  Remove NOREPLACE for /etc/elasticsearch in rpm and deb (elastic#37839)
  Remove "reinstall" packaging tests (elastic#37851)
  Add unit tests for ShardStateAction's ShardStartedClusterStateTaskExecutor (elastic#37756)
  Exit batch files explictly using ERRORLEVEL (elastic#29583)
  TransportUnfollowAction should increase settings version (elastic#37859)
  AsyncTwoPhaseIndexerTests race condition fixed (elastic#37830)
  Do not allow negative variances (elastic#37384)
  ...
pgomulka added a commit to pgomulka/elasticsearch that referenced this pull request Feb 1, 2019
The unlucky timing can cause this test to fail when the indexing is triggered from `maybeTriggerAsyncJob`. As this is asynchronous, in can finish quicker then the test stepping over to next assertion
The introduced barrier solves the problem
closes elastic#37695
pgomulka added a commit that referenced this pull request Feb 4, 2019
The unlucky timing can cause this test to fail when the indexing is triggered from maybeTriggerAsyncJob. As this is asynchronous, in can finish quicker then the test stepping over to next assertion
The introduced barrier solves the problem
closes #37695
Backport #37830
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Core/Infra/Core Core issues without another label >test-failure Triaged test failures from CI v6.7.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[CI] AsyncTwoPhaseIndexerTests.testStateMachine failure
4 participants