Skip to content

Warn of change of default of wait_for_active_shards#67246

Merged
DaveCTurner merged 15 commits intoelastic:7.xfrom
DaveCTurner:2021-01-11-deprecate-wait_for_active_shards-default-for-close-index-7x
Jan 13, 2021
Merged

Warn of change of default of wait_for_active_shards#67246
DaveCTurner merged 15 commits intoelastic:7.xfrom
DaveCTurner:2021-01-11-deprecate-wait_for_active_shards-default-for-close-index-7x

Conversation

@DaveCTurner
Copy link
Copy Markdown
Member

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. Please avoid if you can. 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
Copy Markdown
Collaborator

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

tlrx
tlrx previously approved these changes Jan 11, 2021
@DaveCTurner
Copy link
Copy Markdown
Member 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
Copy Markdown
Member 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
Copy Markdown
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
Copy Markdown
Member 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
Copy Markdown
Member 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. Please avoid if you can. Team:Distributed Meta label for distributed team.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants