Skip to content

Fixes active shard count check in the case of all shards#19760

Merged
abeyad merged 1 commit intoelastic:masterfrom
abeyad:fix/active-shard-count-check
Aug 2, 2016
Merged

Fixes active shard count check in the case of all shards#19760
abeyad merged 1 commit intoelastic:masterfrom
abeyad:fix/active-shard-count-check

Conversation

@abeyad
Copy link
Copy Markdown

@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 DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. labels Aug 2, 2016
Copy link
Copy Markdown
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
Copy Markdown
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
Copy Markdown
Member

dakrone commented Aug 2, 2016

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

@abeyad
Copy link
Copy Markdown
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 DO NOT USE. Use ":Distributed/Indices APIs" or ":StorageEngine/Templates" instead. v5.0.0-alpha5

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants