-
Notifications
You must be signed in to change notification settings - Fork 25k
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
doc: deprecate _primary and _replica shard option #26792
Conversation
@dnhatn we have special asciidoc syntax for deprecations, see: https://github.com/elastic/docs#additions-and-deprecations |
Thanks @dakrone. I will update it. |
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.
I think that we should add deprecation logging for use of these preferences.
retest this please |
Thanks @jasontedor for the suggestion. I have added deprecation logging for these parameters. Could you please have another look? |
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.
I left a few more, almost there.
@@ -199,12 +199,20 @@ private ShardIterator preferenceActiveShardIterator(IndexShardRoutingTable index | |||
case LOCAL: | |||
return indexShard.preferNodeActiveInitializingShardsIt(Collections.singleton(localNodeId)); | |||
case PRIMARY: | |||
deprecationLogger.deprecated("[_primary] has been deprecated in 6.0+, and will be removed in 7.0; " + |
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.
The version here should be 6.1 (we are going to backport this to 6.x but not 6.0).
@@ -9,18 +9,21 @@ The `preference` is a query string parameter which can be set to: | |||
[horizontal] | |||
`_primary`:: | |||
The operation will go and be executed only on the primary | |||
shards. | |||
shards. deprecated[6.0.0, will be removed in 7.0, use `_only_nodes` or `_prefer_nodes`] |
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.
This should be 6.1.0.
@@ -1,5 +1,10 @@ | |||
--- | |||
"List of strings": | |||
- skip: | |||
version: " - 5.99.99" |
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.
Let's only skip to 6.0.99.
@dnhatn Let's be careful when we merge this that the master branch does not break because of the BWC tests that use preferences. If you need help walking through how to test that locally, please let me know. |
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.
LGTM. This looks good, but please see my note about making sure that this doesn't break master because of the BWC tests there. So: a green CI PR build on this is not enough.
@jasontedor The BWC tests are good for this branch. Thanks a lot for your guidance. |
Tells v6 users that we are going to remove these options in v7.
Related #26335