Skip to content

Conversation

@abeyad
Copy link

@abeyad abeyad commented Aug 2, 2016

Fixes the active shard count check in the case of
ActiveShardCount.ALL by checking for active shards,
not just started shards, as a shard could be active
but in the relocating state (i.e. not in the started
state).

Previously, the ReplicationOperationTests#testWaitForActiveShards failed with the following reproduce:

gradle :core:test -Dtests.seed=4AFDAC476714C11 -Dtests.class=org.elasticsearch.action.support.replication.ReplicationOperationTests -Dtests.method="testWaitForActiveShards" -Dtests.security.manager=true -Dtests.locale=ar-SD -Dtests.timezone=Europe/Istanbul

It no longer fails with this PR

@abeyad abeyad added >bug review :Data Management/Indices APIs APIs to create and manage indices and templates labels Aug 2, 2016
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps add a comment to explain why the +1? (because of the primary I assume, but it's still nice to be explicit for newcomers to the codebase)

Copy link
Author

Choose a reason for hiding this comment

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

Will do, good suggestion!

@dakrone
Copy link
Member

dakrone commented Aug 2, 2016

LGTM, left one comment, no need to re-review

@abeyad
Copy link
Author

abeyad commented Aug 2, 2016

@dakrone thanks for the review!

ActiveShardCount.ALL by checking for active shards,
not just started shards, as a shard could be active
but in the relocating state (i.e. not in the started
state).
@abeyad abeyad force-pushed the fix/active-shard-count-check branch from e4846f4 to 082bb5d Compare August 2, 2016 22:00
@abeyad abeyad merged commit c28eee7 into elastic:master Aug 2, 2016
@abeyad abeyad deleted the fix/active-shard-count-check branch August 2, 2016 22:00
jasontedor added a commit to jaymode/elasticsearch that referenced this pull request Aug 3, 2016
* master:
  Fix REST test documentation
  [Test] move methods from bwc test to test package for use in plugins (elastic#19738)
  package-info.java should be in src/main only.
  Split regular histograms from date histograms. elastic#19551
  Tighten up concurrent store metadata listing and engine writes (elastic#19684)
  Plugins: Make NamedWriteableRegistry immutable and add extenion point for named writeables
  Add documentation for the 'elasticsearch-translog' tool
  [TEST] Increase time waiting for all shards to move off/on to a node
  Fixes the active shard count check in the case of (elastic#19760)
  Fixes cat tasks operation in detailed mode
  ignore some docker craziness in scccomp environment checks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

>bug :Data Management/Indices APIs APIs to create and manage indices and templates v5.0.0-alpha5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants