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

Warn of change of default of wait_for_active_shards #67246

Conversation

DaveCTurner
Copy link
Contributor

In 7.x the close indices API defaults to ?wait_for_active_shards=0 but
from 8.0 it will default to respecting the index settings instead. This
commit introduces the index-setting value for this parameter on this
API allowing users to opt-in to the future behaviour today, and starts
to emit a deprecation warning for users that use the default.

Relates #67158
Closes #66419

In 7.x the close indices API defaults to `?wait_for_active_shards=0` but
from 8.0 it will default to respecting the index settings instead. This
commit introduces the `index-setting` value for this parameter on this
API allowing users to opt-in to the future behaviour today, and starts
to emit a deprecation warning for users that use the default.

Relates elastic#67158
Closes elastic#66419
@DaveCTurner DaveCTurner added >deprecation :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. v7.12.0 labels Jan 11, 2021
@DaveCTurner DaveCTurner requested a review from tlrx January 11, 2021 12:04
@elasticmachine elasticmachine added the Team:Distributed Meta label for distributed team label Jan 11, 2021
@elasticmachine
Copy link
Collaborator

Pinging @elastic/es-distributed (Team:Distributed)

tlrx
tlrx previously approved these changes Jan 11, 2021
@DaveCTurner
Copy link
Contributor Author

Sorry about all the iterations here @tlrx, ./gradlew check fails for me for unrelated reasons so I've been trying to run all the relevant-looking things and am relying on CI to run everything else. I'm going to have to port some of these changes to master too. Bear with me.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 12, 2021
Some changes for `master` that make elastic#67246 a bit easier.

Relates elastic#67158
@DaveCTurner DaveCTurner requested a review from tlrx January 12, 2021 18:00
@DaveCTurner
Copy link
Contributor Author

Ok this is good for another look now, sorry again for all the extra noise.

@DaveCTurner DaveCTurner dismissed tlrx’s stale review January 12, 2021 21:20

substantial changes since this approval

Copy link
Member

@tlrx tlrx left a comment

Choose a reason for hiding this comment

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

LGTM

DaveCTurner added a commit that referenced this pull request Jan 13, 2021
Also includes some changes for `master` that make #67246 a bit smaller.

Relates #67158
@DaveCTurner DaveCTurner merged commit 8660e8d into elastic:7.x Jan 13, 2021
@DaveCTurner DaveCTurner deleted the 2021-01-11-deprecate-wait_for_active_shards-default-for-close-index-7x branch January 13, 2021 14:34
DaveCTurner added a commit that referenced this pull request Jan 13, 2021
@DaveCTurner
Copy link
Contributor Author

😭 in 11381c3 I have reverted this because it causes master BWC tests to fail thanks to the generation of warnings that master does not expect. I'll work on a PR to address that in master.

DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 14, 2021
Part of the fixes for elastic#66419, this commit permits nodes to emit the
deprecation warning regarding not specifying `?wait_for_active_shards`
when closing an index in 7.x versions for x ≥ 12. This change is
required on `master` too since the BWC tests encounter these warnings.

Relates elastic#67246, which is the 7.x part of this change.
DaveCTurner added a commit that referenced this pull request Jan 14, 2021
Part of the fixes for #66419, this commit permits nodes to emit the
deprecation warning regarding not specifying `?wait_for_active_shards`
when closing an index in 7.x versions for x ≥ 12. This change is
required on `master` too since the BWC tests encounter these warnings.

Relates #67246, which is the 7.x part of this change.
DaveCTurner added a commit to DaveCTurner/elasticsearch that referenced this pull request Jan 14, 2021
In 7.x the close indices API defaults to `?wait_for_active_shards=0` but
from 8.0 it will default to respecting the index settings instead. This
commit introduces the `index-setting` value for this parameter on this
API allowing users to opt-in to the future behaviour today, and starts
to emit a deprecation warning for users that use the default.

Relates elastic#67158
Retry of elastic#67246 now that elastic#67498 is merged to `master`
Closes elastic#66419
@DaveCTurner
Copy link
Contributor Author

I have opened #67527 to undo the revert and reinstate this commit.

DaveCTurner added a commit that referenced this pull request Jan 14, 2021
In 7.x the close indices API defaults to `?wait_for_active_shards=0` but
from 8.0 it will default to respecting the index settings instead. This
commit introduces the `index-setting` value for this parameter on this
API allowing users to opt-in to the future behaviour today, and starts
to emit a deprecation warning for users that use the default.

Relates #67158
Retry of #67246 now that #67498 is merged to `master`
Closes #66419
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
>deprecation :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. Team:Distributed Meta label for distributed team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants