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

Add wait_for_active_shards parameter to index open command #26682

Merged
merged 3 commits into from
Sep 22, 2017

Conversation

alexshadow007
Copy link
Contributor

Closes #20937

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@alexshadow007 alexshadow007 changed the title Add wait_for_shards parameter to index open command Add wait_for_active_shards parameter to index open command Sep 17, 2017
Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

I left two minor documentation comments, otherwise looks good to me.

We can change the default of only waiting for the primary shards
to start through the request parameter `wait_for_active_shards`.
A detailed explanation of `wait_for_active_shards` and its possible values
can be found <<index-wait-for-active-shards,here>>.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it's better to link to the "Wait For Active Shards" section of the "Create index" command. The section linked to here is about the same name parameter that is used when indexing documents.

Also I think this section can be shorter and just link to index creation, e.g.:

[float]
=== Wait For Active Shards

Because opening an index allocates its shards, the
<<create-index-wait-for-active-shards,`wait_for_active_shards`>> setting on
index creation applies to the index opening action as well.

},
"wait_for_active_shards": {
"type" : "string",
"description" : "Sets the number of shard copies that must be active before proceeding with the index operation. Defaults to 1, meaning the primary shard only. Set to `all` for all shard copies, otherwise set to any non-negative value less than or equal to the total number of copies for the shard (number of replicas + 1)"
Copy link
Contributor

Choose a reason for hiding this comment

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

I think you copied this from the wrong location (this is the description that applies to the indexing command, not the index creation command). That one just states

"wait_for_active_shards": {
          "type" : "string",
          "description" : "Set the number of active shards to wait for before the operation returns." 
        },

@ywelsch ywelsch added v6.1.0 v7.0.0 :Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >enhancement labels Sep 19, 2017
@alexshadow007
Copy link
Contributor Author

@ywelsch Done

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Thank you @alexshadow007

@ywelsch
Copy link
Contributor

ywelsch commented Sep 20, 2017

@elasticmachine test this please

Copy link
Contributor

@ywelsch ywelsch left a comment

Choose a reason for hiding this comment

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

Running the tests, I noticed the PR still needs to implement a BWC layer for interoperability with ES versions that won't have this commit. Can you add that?

@alexshadow007
Copy link
Contributor Author

@ywelsch Done

@ywelsch
Copy link
Contributor

ywelsch commented Sep 21, 2017

@elasticmachine retest this please

@ywelsch ywelsch removed the v6.1.0 label Sep 22, 2017
@ywelsch ywelsch merged commit ff737a8 into elastic:master Sep 22, 2017
@ywelsch
Copy link
Contributor

ywelsch commented Sep 22, 2017

Thanks @alexshadow007. I will see about backporting this (and adding a note to the Breaking changes documentation as we now wait for the primaries to be allocated before the command returns).

ywelsch added a commit that referenced this pull request Sep 22, 2017
…mmand

This commit disables BWC tests while adding a v6.1 BWC layer for the PR #26682
ywelsch pushed a commit that referenced this pull request Sep 22, 2017
Adds the wait_for_active_shards parameter to the index open command. Similar to the index creation command, the index open command will now, by default, wait until the primaries have been allocated.

Closes #20937
@ywelsch ywelsch added the v6.1.0 label Sep 22, 2017
@ywelsch
Copy link
Contributor

ywelsch commented Sep 22, 2017

I've backported this to the 6.1 branch, but changed the default of the wait_for_active_shards parameter there to 0, so that there is no change of default behavior between 6.0 and 6.1.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
:Distributed/CRUD A catch all label for issues around indexing, updating and getting a doc by id. Not search. >enhancement v6.1.0 v7.0.0-beta1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants