Allow for queries on _tier to skip shards during coordinator rewrite #114990

andreidan · 2024-10-17T11:18:04Z

The _tier metadata field was not used on the
coordinator when rewriting queries in order to
exclude shards that don't match. This lead to queries
in the following form to continue to report failures
even though the only unavailable shards were in the
tier that was excluded from search (frozen tier in this
example):

POST testing/_search
{
  "query": {
    "bool": {
      "must_not": [
        {
          "term": {
            "_tier": "data_frozen"
          }
        }
      ]
    }
  }
}

This PR addresses this by having the queries that can
execute on _tier (term, match, prefix, wildcard) execute
a coordinator rewrite to exclude the indices that don't match
the _tier query before attempting to reach to the shards
(shards, that might not be available and raise errors).

Fixes #114910

…ally mounted index

andreidan · 2024-10-17T12:55:13Z

@elasticmachine update branch

andreidan · 2024-10-17T19:49:16Z

@elasticmachine update branch

andreidan · 2024-10-18T10:13:29Z

@elasticmachine update branch

andreidan · 2024-10-18T10:23:33Z

x-pack/plugin/core/src/main/java/module-info.java

@@ -34,7 +34,6 @@
    exports org.elasticsearch.protocol.xpack;
    exports org.elasticsearch.snapshots.sourceonly;
    exports org.elasticsearch.xpack.cluster.action;
-    exports org.elasticsearch.xpack.cluster.routing.allocation.mapper;


This is now in server

andreidan · 2024-10-18T10:23:44Z

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/XPackPlugin.java

@@ -303,11 +301,6 @@ private static boolean alreadyContainsXPackCustomMetadata(ClusterState clusterSt
            || metadata.custom(TransformMetadata.TYPE) != null;
    }

-    @Override


Moved to server

javanna

This looks great! I left a rather important comment on the logic for match query, but I am good with this going in without additional reviews once the last comments are addressed

server/src/main/java/org/elasticsearch/index/query/CoordinatorRewriteContext.java

javanna · 2024-10-23T18:56:25Z

server/src/main/java/org/elasticsearch/index/query/CoordinatorRewriteContextProvider.java

+        // rewrite only based on the _tier field) and we might decide to remove this artificial
+        // limitation to enable coordinator rewrites based on _tier for hot and warm indices
+        // (currently the _tier coordinator rewrite is only available for mounted and partially mounted
+        // indices)


perhaps make it super clear that the range info will only have a value for read only indices, because those are the only indices for which that info is populated in the cluster state

I believe read-only indices is a superset of where we actually have this optimization available - i.e. only for fully and partially mounted indices ?
I believe the comment is accurate if my assumption is correct (i.e. hot/warm indices don't have this optimization, cold/frozen ones - fully/partially mounted ones - do) but happy to clarify further if you have a better suggestion?

javanna · 2024-10-23T18:58:26Z

server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java

+            return tier.isEmpty() == false ? tier : null;
+        }
+        Settings settings = context.getIndexSettings().getSettings();
+        String value = DataTier.TIER_PREFERENCE_SETTING.get(settings);


I find it surprising that how we read the index setting here differs from how we read it above in CoordinatorRewriteContextProvider. Why is that? Reading the tier from index settings should require the same logic in those two places?

We've had performance issues in the past with parsing the tier preference from the settings. We now have a version of the IndexMetadata that does the parsing and caches the result for the lifetime of the IndexMetadata.
Since the CoordinatorRewriteContextProvider has access to the IndexMetadata I preferred to take this route rather than re-parsing the setting.

server/src/main/java/org/elasticsearch/index/query/QueryRewriteContext.java

server/src/main/java/org/elasticsearch/index/query/MatchQueryBuilder.java

andreidan · 2024-10-24T08:59:50Z

@elasticmachine update branch

elasticsearchmachine · 2024-10-24T10:43:13Z

💚 Backport successful

Status	Branch	Result
✅	8.16
✅	8.x

…lastic#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes elastic#114910

…ewrite (#114990) (#115513) * Allow for queries on _tier to skip shards during coordinator rewrite (#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes #114910 * Don't use getFirst * Test compile

…lastic#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes elastic#114910

…write (#114990) (#115514) * Allow for queries on _tier to skip shards during coordinator rewrite (#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes #114910 * Don't use getFirst * test compilation --------- Co-authored-by: Elastic Machine <[email protected]>

…lastic#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes elastic#114910

…nted indices) (#115797) As part of #114990 we enabled using the `_tier` field as part of the coordinator rewrite in order to skip shards that do not match a `_tier` filter, but only for fully/partially mounted indices. This PR enhances the previous work by allowing a coordinator rewrite to skip shards that will not match the `_tier` query for all indices (irrespective of their lifecycle state i.e. hot and warm indices can now skip shards based on the `_tier` query) Note however, that hot/warm indices will not automatically take advantage of the `can_match` coordinator rewrite (like read only indices do) but only the search requests that surpass the `pre_filter_shard_size` threshold will. Relates to [#114910](#114910)

…nted indices) (elastic#115797) As part of elastic#114990 we enabled using the `_tier` field as part of the coordinator rewrite in order to skip shards that do not match a `_tier` filter, but only for fully/partially mounted indices. This PR enhances the previous work by allowing a coordinator rewrite to skip shards that will not match the `_tier` query for all indices (irrespective of their lifecycle state i.e. hot and warm indices can now skip shards based on the `_tier` query) Note however, that hot/warm indices will not automatically take advantage of the `can_match` coordinator rewrite (like read only indices do) but only the search requests that surpass the `pre_filter_shard_size` threshold will. Relates to [elastic#114910](elastic#114910) (cherry picked from commit 71dfb06) Signed-off-by: Andrei Dan <[email protected]>

…st mounted indices) (#115797) (#116076) * Enable _tier based coordinator rewrites for all indices (not just mounted indices) (#115797) As part of #114990 we enabled using the `_tier` field as part of the coordinator rewrite in order to skip shards that do not match a `_tier` filter, but only for fully/partially mounted indices. This PR enhances the previous work by allowing a coordinator rewrite to skip shards that will not match the `_tier` query for all indices (irrespective of their lifecycle state i.e. hot and warm indices can now skip shards based on the `_tier` query) Note however, that hot/warm indices will not automatically take advantage of the `can_match` coordinator rewrite (like read only indices do) but only the search requests that surpass the `pre_filter_shard_size` threshold will. Relates to [#114910](#114910) (cherry picked from commit 71dfb06) Signed-off-by: Andrei Dan <[email protected]> * Fix test compilation --------- Co-authored-by: Elastic Machine <[email protected]>

…lastic#114990) The `_tier` metadata field was not used on the coordinator when rewriting queries in order to exclude shards that don't match. This lead to queries in the following form to continue to report failures even though the only unavailable shards were in the tier that was excluded from search (frozen tier in this example): ``` POST testing/_search { "query": { "bool": { "must_not": [ { "term": { "_tier": "data_frozen" } } ] } } } ``` This PR addresses this by having the queries that can execute on `_tier` (term, match, query string, simple query string, prefix, wildcard) execute a coordinator rewrite to exclude the indices that don't match the `_tier` query **before** attempting to reach to the shards (shards, that might not be available and raise errors). Fixes elastic#114910

…nted indices) (elastic#115797) As part of elastic#114990 we enabled using the `_tier` field as part of the coordinator rewrite in order to skip shards that do not match a `_tier` filter, but only for fully/partially mounted indices. This PR enhances the previous work by allowing a coordinator rewrite to skip shards that will not match the `_tier` query for all indices (irrespective of their lifecycle state i.e. hot and warm indices can now skip shards based on the `_tier` query) Note however, that hot/warm indices will not automatically take advantage of the `can_match` coordinator rewrite (like read only indices do) but only the search requests that surpass the `pre_filter_shard_size` threshold will. Relates to [elastic#114910](elastic#114910)

andreidan added 7 commits October 16, 2024 15:50

Add tier to CoordinatorRewriteContext and test to query missing parti…

0f2d1f9

…ally mounted index

The _tier field is exposed in server

efba43c

Only match _tier on the coordinator if _tier specified in term query

997b4ef

Fix tests

f19c257

rename getter

b44c9b2

minor refactoring and tests

4717b82

minor refactoring and tests

e61f0a0

andreidan added WIP :Search Foundations/Search Catch all for Search Foundations labels Oct 17, 2024

elasticsearchmachine added the v9.0.0 label Oct 17, 2024

elasticmachine and others added 4 commits October 17, 2024 14:55

Merge branch 'main' into can-match-doesnt-fail-on-frozen

98277e1

Add bool query and match_all when _tier matches

26a572a

spotless

3c70b1f

trim things

c3bc4db

elasticmachine and others added 2 commits October 17, 2024 21:49

Merge branch 'main' into can-match-doesnt-fail-on-frozen

8510222

More queries and tests

a1d3bfd

elasticmachine and others added 3 commits October 18, 2024 12:13

Merge branch 'main' into can-match-doesnt-fail-on-frozen

d07dc7c

spotless

350a2c9

This is not needed as tier mapper tests are now in server

4bd9cce

andreidan commented Oct 18, 2024

View reviewed changes

andreidan changed the title ~~Can match phase doesnt fail on frozen~~ Allow for querries on _tier to skip shards in the can_match phase Oct 18, 2024

andreidan marked this pull request as ready for review October 18, 2024 10:34

andreidan added >bug v8.15.4 v8.16.1 and removed WIP labels Oct 18, 2024

javanna approved these changes Oct 23, 2024

View reviewed changes

javanna changed the title ~~Allow for querries on _tier to skip shards in the can_match phase~~ Allow for queries on _tier to skip shards in the can_match phase Oct 23, 2024

javanna changed the title ~~Allow for queries on _tier to skip shards in the can_match phase~~ Allow for queries on _tier to skip shards during coordinator rewrite Oct 23, 2024

andreidan added 4 commits October 24, 2024 09:45

Make static method an instance method

f9b2913

UnsupportedOperationException where needed

3e152f1

Remove coordinator rewrite support from match query

3f87982

spotless

8b7daf3

Merge branch 'main' into can-match-doesnt-fail-on-frozen

fc3818c

andreidan added auto-backport Automatically create backport pull requests when merged auto-merge-without-approval Automatically merge pull request when CI checks pass (NB doesn't wait for reviews!) labels Oct 24, 2024

Drop empty line

929a916

elasticsearchmachine merged commit 327f232 into elastic:main Oct 24, 2024
16 checks passed

andreidan deleted the can-match-doesnt-fail-on-frozen branch October 24, 2024 10:42

This was referenced Oct 24, 2024

[8.16] Allow for queries on _tier to skip shards during coordinator rewrite (#114990) #115513

Merged

[8.x] Allow for queries on _tier to skip shards during coordinator rewrite (#114990) #115514

Merged

andreidan mentioned this pull request Oct 30, 2024

Enable _tier based coordinator rewrites for all indices (not just mounted indices) #115797

Merged

andreidan mentioned this pull request Nov 1, 2024

[8.x] Enable _tier based coordinator rewrites for all indices (not just mounted indices) (#115797) #116076

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow for queries on _tier to skip shards during coordinator rewrite #114990

Allow for queries on _tier to skip shards during coordinator rewrite #114990

andreidan commented Oct 17, 2024 •

edited

Loading

andreidan commented Oct 17, 2024

andreidan commented Oct 17, 2024

andreidan commented Oct 18, 2024

andreidan Oct 18, 2024

andreidan Oct 18, 2024

javanna left a comment

javanna Oct 23, 2024

andreidan Oct 24, 2024

javanna Oct 23, 2024

andreidan Oct 24, 2024

andreidan commented Oct 24, 2024

elasticsearchmachine commented Oct 24, 2024

Allow for queries on _tier to skip shards during coordinator rewrite #114990

Allow for queries on _tier to skip shards during coordinator rewrite #114990

Conversation

andreidan commented Oct 17, 2024 • edited Loading

andreidan commented Oct 17, 2024

andreidan commented Oct 17, 2024

andreidan commented Oct 18, 2024

andreidan Oct 18, 2024

Choose a reason for hiding this comment

andreidan Oct 18, 2024

Choose a reason for hiding this comment

javanna left a comment

Choose a reason for hiding this comment

javanna Oct 23, 2024

Choose a reason for hiding this comment

andreidan Oct 24, 2024

Choose a reason for hiding this comment

javanna Oct 23, 2024

Choose a reason for hiding this comment

andreidan Oct 24, 2024

Choose a reason for hiding this comment

andreidan commented Oct 24, 2024

elasticsearchmachine commented Oct 24, 2024

💚 Backport successful

andreidan commented Oct 17, 2024 •

edited

Loading