Improve docs for search preferences #32098

DaveCTurner · 2018-07-16T15:29:18Z

Today it is unclear what guarantees are offered by the search preference
feature, and we claim a guarantee that is stronger than what we really offer:

A custom value will be used to guarantee that the same shards will be used
for the same custom value.

This commit clarifies this documentation and explains more clearly why
_primary, _replica, etc. are deprecated in 6.x and removed in master.

Relates #31929 #26335 #26791.

Today it is unclear what guarantees are offered by the search preference feature, and we claim a guarantee that is stronger than what we really offer: > A custom value will be used to guarantee that the same shards will be used > for the same custom value. This commit clarifies this documentation and explains more clearly why `_primary`, `_replica`, etc. are deprecated in `6.x` and removed in `master`. Relates elastic#31929 elastic#26335 elastic#26791.

elasticmachine · 2018-07-16T15:29:20Z

Pinging @elastic/es-distributed

elasticmachine · 2018-07-16T15:29:21Z

Pinging @elastic/es-search-aggs

DaveCTurner · 2018-07-16T15:32:12Z

This change is intended to be applied to 6.x and backported as far as 6.1, then I'll open a separate one to forward-port to master (removing the irrelevant bits).

bleskes

thx David. I left some comments.

bleskes · 2018-07-17T11:28:19Z

docs/reference/search/request/preference.asciidoc

+searches to certain sets of shard copies, for instance to make better use of
+per-copy caches.
+
+Preferences do not _guarantee_ that any particular shard copies are used in a


we need to be more careful here. Some preferences are guaranteed (like _only_local). These are useful for debugging and not production use.

Really it's just _only_local (undocumented prior to this change, and I wrote this paragraph before adding that 😁) I reworded this in cae2225 and moved it below the description of the options where it flowed better.

bleskes · 2018-07-17T11:29:37Z

docs/reference/search/request/preference.asciidoc

-	shards. deprecated[6.1.0, will be removed in 7.0, use `_only_nodes` or `_prefer_nodes`]
+`_primary`::
+	The operation will be executed only on primary shards.
+	deprecated[6.1.0, will be removed in 7.0, use `_only_nodes` or


I thought we were also going to explain why we removed it - in this case because it puts and unreasonable load on the primary with no gains as the ES replication is sync

nevermind, I see it now at the end. maybe add here something like "see end of page for more information"

Ok, done. I also recommended _only_nodes, _prefer_nodes or a custom string value there instead.

bleskes · 2018-07-17T11:31:49Z

docs/reference/search/request/preference.asciidoc

+	Any value that does not start with `_`. If two searches both give the same
+	custom string value for their preference and the underlying cluster state
+	does not change then the same ordering of shards will be used for the
+	searches. This does not guarantee that the exact same shards will be used


do we want to say something like "That said, we expect this selection to be stable for a long period of time. This allows users of a customized value to optimize cache usage"?

Ok, I did that in cae2225.

DaveCTurner

Addressed feedback.

DaveCTurner · 2018-07-17T12:23:55Z

docs/reference/search/request/preference.asciidoc

+searches to certain sets of shard copies, for instance to make better use of
+per-copy caches.
+
+Preferences do not _guarantee_ that any particular shard copies are used in a


Really it's just _only_local (undocumented prior to this change, and I wrote this paragraph before adding that 😁) I reworded this in cae2225 and moved it below the description of the options where it flowed better.

DaveCTurner · 2018-07-17T12:34:39Z

docs/reference/search/request/preference.asciidoc

-	shards. deprecated[6.1.0, will be removed in 7.0, use `_only_nodes` or `_prefer_nodes`]
+`_primary`::
+	The operation will be executed only on primary shards.
+	deprecated[6.1.0, will be removed in 7.0, use `_only_nodes` or


Ok, done. I also recommended _only_nodes, _prefer_nodes or a custom string value there instead.

DaveCTurner · 2018-07-17T12:34:57Z

docs/reference/search/request/preference.asciidoc

+	Any value that does not start with `_`. If two searches both give the same
+	custom string value for their preference and the underlying cluster state
+	does not change then the same ordering of shards will be used for the
+	searches. This does not guarantee that the exact same shards will be used


Ok, I did that in cae2225.

bleskes · 2018-07-17T13:48:18Z

docs/reference/search/request/preference.asciidoc

@@ -65,3 +85,22 @@ GET /_search?preference=xyzabc123
 ------------------------------------------------
 // CONSOLE

+NOTE: The `_only_local` preference guarantees only to use shard copies on the
+local node, which is sometimes useful for troubleshooting. All other options do
+not _fully_ guarantee that any particular shard copies are used in a search,


I think _only_nodes has the same so you won't need to route your request to the node in question

bleskes

LGTM

Today it is unclear what guarantees are offered by the search preference feature, and we claim a guarantee that is stronger than what we really offer: > A custom value will be used to guarantee that the same shards will be used > for the same custom value. This commit clarifies this documentation and explains more clearly why `_primary`, `_replica`, etc. are deprecated in `6.x` and removed in `master`. Relates #31929 #26335 #26791.

Today it is unclear what guarantees are offered by the search preference feature, and we claim a guarantee that is stronger than what we really offer: > A custom value will be used to guarantee that the same shards will be used > for the same custom value. This commit clarifies this documentation. Forward-port of elastic#32098 to `master`.

Today it is unclear what guarantees are offered by the search preference feature, and we claim a guarantee that is stronger than what we really offer: > A custom value will be used to guarantee that the same shards will be used > for the same custom value. This commit clarifies this documentation. Forward-port of #32098 to `master`.

* 6.x: Fix rollup on date fields that don't support epoch_millis (#31890) Revert "Introduce a Hashing Processor (#31087)" (#32179) [test] use randomized runner in packaging tests (#32109) Painless: Fix caching bug and clean up addPainlessClass. (#32142) Fix BwC Tests looking for UUID Pre 6.4 (#32158) (#32169) Call setReferences() on custom referring tokenfilters in _analyze (#32157) Add more contexts to painless execute api (#30511) Add EC2 credential test for repository-s3 (#31918) Fix CP for namingConventions when gradle home has spaces (#31914) Convert Version to Java - clusterformation part1 (#32009) Fix Java 11 javadoc compile problem Improve docs for search preferences (#32098) Configurable password hashing algorithm/cost(#31234) (#32092) [DOCS] Update TLS on Docker for 6.3 ESIndexLevelReplicationTestCase doesn't support replicated failures but it's good to know what they are Switch distribution to new style Requests (#30595) Build: Skip jar tests if jar disabled Build: Move shadow customizations into common code (#32014) Painless: Add PainlessClassBuilder (#32141) Fix accidental duplication of bwc test for script behavior Handle missing values in painless (#30975) (#31903) Build: Make additional test deps of check (#32015) Painless: Fix Bug with Duplicate PainlessClasses (#32110) Adjust translog after versionType removed in 7.0 (#32020) Disable C2 from using AVX-512 on JDK 10 (#32138) [Rollup] Add new capabilities endpoint for concrete rollup indices (#32111) Mute :qa:mixed-cluster indices.stats/10_index/Index - all’ [ML] Wait for aliases in multi-node tests (#32086) Ensure to release translog snapshot in primary-replica resync (#32045) Docs: Fix missing example script quote (#32010) Add Index UUID to `/_stats` Response (#31871) (#32113) [ML] Move analyzer dependencies out of categorization config (#32123) [ML][DOCS] Add missing 6.3.0 release notes (#32099) Updates the build to gradle 4.9 (#32087) Update monitoring template version to 6040099 (#32088) Fix put mappings java API documentation (#31955) Add exclusion option to `keep_types` token filter (#32012)

DaveCTurner added >docs General docs changes :Search/Search Search-related issues that do not fall into other categories :Distributed/Distributed A catch all label for anything in the Distributed Area. If you aren't sure, use this one. v6.1.5 v6.2.5 v6.4.0 v6.3.2 labels Jul 16, 2018

DaveCTurner requested a review from bleskes July 16, 2018 15:29

bleskes reviewed Jul 17, 2018

View reviewed changes

Review feedback

cae2225

DaveCTurner commented Jul 17, 2018

View reviewed changes

Merge branch '6.x' into 2018-07-16-preference-docs

872b51b

bleskes reviewed Jul 17, 2018

View reviewed changes

bleskes approved these changes Jul 17, 2018

View reviewed changes

DaveCTurner merged commit 4e5f389 into elastic:6.x Jul 18, 2018

DaveCTurner deleted the 2018-07-16-preference-docs branch July 18, 2018 08:41

DaveCTurner mentioned this pull request Jul 18, 2018

Improve docs for search preferences #32159

Merged

DaveCTurner mentioned this pull request Jul 30, 2018

Deprecating _primary preference makes getting consistent results impossible(?) #31929

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve docs for search preferences #32098

Improve docs for search preferences #32098

DaveCTurner commented Jul 16, 2018

elasticmachine commented Jul 16, 2018

elasticmachine commented Jul 16, 2018

DaveCTurner commented Jul 16, 2018

bleskes left a comment

bleskes Jul 17, 2018

DaveCTurner Jul 17, 2018

bleskes Jul 17, 2018

bleskes Jul 17, 2018

DaveCTurner Jul 17, 2018

bleskes Jul 17, 2018

DaveCTurner Jul 17, 2018

DaveCTurner left a comment

DaveCTurner Jul 17, 2018

DaveCTurner Jul 17, 2018

DaveCTurner Jul 17, 2018

bleskes Jul 17, 2018

bleskes left a comment

Improve docs for search preferences #32098

Improve docs for search preferences #32098

Conversation

DaveCTurner commented Jul 16, 2018

elasticmachine commented Jul 16, 2018

elasticmachine commented Jul 16, 2018

DaveCTurner commented Jul 16, 2018

bleskes left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

DaveCTurner left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bleskes left a comment

Choose a reason for hiding this comment