-
Notifications
You must be signed in to change notification settings - Fork 24.8k
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
Parameter improvements to Cluster Health API wait for shards #20223
Conversation
Previously, the cluster health API used a strictly numeric value for `wait_for_active_shards`. However, with the introduction of ActiveShardCount and the removal of write consistency level for replication operations, `wait_for_active_shards` is used for write operations to represent values for ActiveShardCount. This commit moves the cluster health API's usage of `wait_for_active_shards` to be consistent with its usage in the write operation APIs. This commit also changes `wait_for_relocating_shards` from a numeric value to a simple boolean value `wait_for_no_relocating_shards` to set whether the cluster health operation should wait for all relocating shards to complete relocation.
@elasticmachine retest this please |
this.waitForRelocatingShards = waitForRelocatingShards; | ||
/** | ||
* Sets whether the request should wait for there to be no relocating shards before | ||
* retrieving the cluster health status. Defaults to <code>false</code>, meaning the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
{@code false}
?
Left a few minor things, mostly documentation wording suggestions. LGTM. |
LGTM |
I'm OK with this change, but I would add a check for it'd be better to make REST param parsing strict (#14719) rather than dealing with these on a case-by-case basis |
@clintongormley I pushed a commit to check for the old param and throw an exception: e4e946b I agree that it would be good to have a general solution to making REST param parsing strict |
So in our use case when we create an index we want to block client calls until all shards are created but we don't care about replicas. So I guess with this change I would need to specify |
@niemyjski you wouldn't need to specify anything, because in 5.0, by default index creation will not return until all of the primary shards are active (see #18985). Suppose you had 2 replicas in addition to your primary. If you wanted to wait for at least 1 replica to become active in addition to the primaries before proceeding with the write, then you would use |
Awesome! I didn't get that from reading the docs, that's good to know! |
@niemyjski if you have any suggestion on how to improve the docs, please let us know (just click that edit button on the website :)) |
Previously, the cluster health API used a strictly numeric value for
wait_for_active_shards
. However, with the introduction of ActiveShardCount and the removal of write consistency level for replication operations,wait_for_active_shards
is used for write operations to represent values for ActiveShardCount. This PR moves the cluster health API's usage ofwait_for_active_shards
to be consistent with its usage in the write operation APIs.This PR also changes
wait_for_relocating_shards
from a numeric value to a simple boolean valuewait_for_no_relocating_shards
to set whether the cluster health operation should wait forall relocating shards to complete relocation.
Breaking changes summary:
wait_for_active_shards
: this is not a breaking change because the same numeric values that were allowed before are allowed now. We have just added the ability to specifyall
as well, in conformity with the the write operation APIs that also usewait_for_active_shards
wait_for_no_relocating_shards
: this is a breaking change because it differs from the old namewait_for_relocating_shards
and it takes a boolean value to represent whether we should wait for the cluster to have no relocations, whereas the oldwait_for_relocating_shards
took a number to represent how many relocating shards to wait on (if this value was set, it was pretty much always set to0
)Closes #20216