-
Notifications
You must be signed in to change notification settings - Fork 25.8k
Add CCS Remote Views Detection #143384
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
Add CCS Remote Views Detection #143384
Changes from all commits
fd94f63
8695671
49c8c04
928cfbf
1c2684a
63da5df
8c6a38e
97294d3
1e95e16
edfe188
ca418b6
a67102a
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,5 @@ | ||
| area: ES|QL | ||
| issues: [] | ||
| pr: 143384 | ||
| summary: Add CCS Remote Views Detection | ||
| type: enhancement |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,65 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.action.fieldcaps; | ||
|
|
||
| import org.elasticsearch.ElasticsearchException; | ||
| import org.elasticsearch.common.io.stream.StreamInput; | ||
| import org.elasticsearch.rest.RestStatus; | ||
| import org.elasticsearch.transport.RemoteClusterAware; | ||
|
|
||
| import java.io.IOException; | ||
| import java.util.List; | ||
| import java.util.stream.Collectors; | ||
| import java.util.stream.Stream; | ||
|
|
||
| /** | ||
| * Thrown when ES|QL detects views during cross-cluster search field resolution. | ||
| * Views are not supported in CCS and the query must fail. | ||
| */ | ||
| public class RemoteViewNotSupportedException extends ElasticsearchException { | ||
|
|
||
| private static final String VIEW_NAMES_KEY = "es.esql.view.names"; | ||
|
|
||
| @SuppressWarnings("this-escape") | ||
| public RemoteViewNotSupportedException(List<String> views) { | ||
| super(message(views)); | ||
| addMetadata(VIEW_NAMES_KEY, views); | ||
| } | ||
|
|
||
| public RemoteViewNotSupportedException(StreamInput in) throws IOException { | ||
| super(in); | ||
| } | ||
|
|
||
| /** | ||
| * Merge two exceptions into one that reports all matched views. | ||
| */ | ||
| public static RemoteViewNotSupportedException merge(RemoteViewNotSupportedException a, RemoteViewNotSupportedException b) { | ||
| return new RemoteViewNotSupportedException( | ||
| Stream.concat(a.getMetadata(VIEW_NAMES_KEY).stream(), b.getMetadata(VIEW_NAMES_KEY).stream()).toList() | ||
| ); | ||
| } | ||
|
|
||
| private static String message(List<String> views) { | ||
| String exclusions = views.stream().map(v -> { | ||
| String[] clusterAndIndex = RemoteClusterAware.splitIndexName(v); | ||
| return clusterAndIndex[0] + ":-" + clusterAndIndex[1]; | ||
| }).collect(Collectors.joining(",")); | ||
| return "ES|QL queries with remote views are not supported. Matched " | ||
| + views | ||
| + ". Remove them from the query pattern or exclude them with [" | ||
| + exclusions | ||
| + "] if matched by a wildcard."; | ||
| } | ||
|
|
||
| @Override | ||
| public RestStatus status() { | ||
| return RestStatus.BAD_REQUEST; | ||
| } | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1 @@ | ||
| 9315000 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1 +1 @@ | ||
| inference_endpoint_metadata_display_model_creator_added,9314000 | ||
| esql_resolve_fields_response_views,9315000 |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -8,6 +8,7 @@ | |
| package org.elasticsearch.xpack.esql.ccq; | ||
|
|
||
| import org.elasticsearch.test.cluster.ElasticsearchCluster; | ||
| import org.elasticsearch.test.cluster.FeatureFlag; | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added a test here to make sure this works end-to-end.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I guess the test you added was
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes! |
||
| import org.elasticsearch.test.cluster.local.distribution.DistributionType; | ||
| import org.elasticsearch.test.cluster.util.Version; | ||
|
|
||
|
|
@@ -30,6 +31,7 @@ static ElasticsearchCluster remoteCluster(Map<String, String> additionalSettings | |
| .setting("node.roles", "[data,ingest,master]") | ||
| .setting("xpack.security.enabled", "false") | ||
| .setting("xpack.license.self_generated.type", "trial") | ||
| .feature(FeatureFlag.ESQL_VIEWS) | ||
| .shared(true); | ||
| if (supportRetryOnShardFailures(version) == false) { | ||
| cluster.setting("cluster.routing.rebalance.enable", "none"); | ||
|
|
@@ -73,6 +75,7 @@ public static ElasticsearchCluster localCluster( | |
| .setting("cluster.remote.remote_cluster.seeds", () -> "\"" + remoteCluster.getTransportEndpoint(0) + "\"") | ||
| .setting("cluster.remote.connections_per_cluster", "1") | ||
| .setting("cluster.remote." + REMOTE_CLUSTER_NAME + ".skip_unavailable", skipUnavailable.toString()) | ||
| .feature(FeatureFlag.ESQL_VIEWS) | ||
| .shared(true); | ||
| if (supportRetryOnShardFailures(version) == false) { | ||
| cluster.setting("cluster.routing.rebalance.enable", "none"); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the Elastic License | ||
| * 2.0; you may not use this file except in compliance with the Elastic License | ||
| * 2.0. | ||
| */ | ||
|
|
||
| package org.elasticsearch.xpack.esql.action; | ||
|
|
||
| import org.elasticsearch.ExceptionsHelper; | ||
| import org.elasticsearch.cluster.metadata.View; | ||
| import org.elasticsearch.core.TimeValue; | ||
| import org.elasticsearch.xpack.esql.view.PutViewAction; | ||
| import org.junit.Before; | ||
|
|
||
| import java.io.IOException; | ||
|
|
||
| import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertAcked; | ||
| import static org.hamcrest.Matchers.containsString; | ||
|
|
||
| public class CrossClusterViewIT extends AbstractCrossClusterTestCase { | ||
|
|
||
| @Before | ||
| public void setupClustersAndViews() throws IOException { | ||
| setupClusters(3); | ||
| createViewOnCluster(REMOTE_CLUSTER_1, "logs-web", "FROM logs-2 | LIMIT 10"); | ||
| createViewOnCluster(REMOTE_CLUSTER_1, "logs-mobile", "FROM logs-2 | LIMIT 10"); | ||
| } | ||
|
|
||
| public void testRemoteViewWildcardMatchFailsQuery() { | ||
| Exception e = expectThrows(Exception.class, () -> runQuery("FROM cluster-a:logs-*", null)); | ||
| Throwable cause = ExceptionsHelper.unwrapCause(e); | ||
| assertThat( | ||
| cause.getMessage(), | ||
| containsString( | ||
| "ES|QL queries with remote views are not supported. Matched [cluster-a:logs-mobile, cluster-a:logs-web]." | ||
| + " Remove them from the query pattern or exclude them with" | ||
| + " [cluster-a:-logs-mobile,cluster-a:-logs-web] if matched by a wildcard." | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| public void testRemoteViewConcreteMatchFailsQuery() { | ||
| Exception e = expectThrows(Exception.class, () -> runQuery("FROM cluster-a:logs-web", null)); | ||
| Throwable cause = ExceptionsHelper.unwrapCause(e); | ||
| assertThat( | ||
| cause.getMessage(), | ||
| containsString( | ||
| "ES|QL queries with remote views are not supported. Matched [cluster-a:logs-web]." | ||
| + " Remove them from the query pattern or exclude them with [cluster-a:-logs-web] if matched by a wildcard." | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| public void testRemoteViewExcludedSucceeds() { | ||
| try (var resp = runQuery("FROM cluster-a:logs-*,cluster-a:-logs-web,cluster-a:-logs-mobile", null)) { | ||
| assertNotNull(resp); | ||
| } | ||
| } | ||
|
|
||
| public void testAllViewsOnRemoteExcludedSucceeds() { | ||
| try (var resp = runQuery("FROM cluster*:logs-*,-cluster-a:*,remote-b:*", null)) { | ||
| assertNotNull(resp); | ||
| } | ||
| } | ||
|
|
||
| public void testRemoteViewFailsOnOneCluster() { | ||
| Exception e = expectThrows(Exception.class, () -> runQuery("FROM cluster-a:logs-*,remote-b:logs-*", null)); | ||
| Throwable cause = ExceptionsHelper.unwrapCause(e); | ||
| assertThat( | ||
| cause.getMessage(), | ||
| containsString( | ||
| "ES|QL queries with remote views are not supported. Matched [cluster-a:logs-mobile, cluster-a:logs-web]." | ||
| + " Remove them from the query pattern or exclude them with" | ||
| + " [cluster-a:-logs-mobile,cluster-a:-logs-web] if matched by a wildcard." | ||
| ) | ||
| ); | ||
| } | ||
|
|
||
| public void testNoRemoteViewsQuerySucceeds() { | ||
| try (var resp = runQuery("FROM remote-b:logs-*", null)) { | ||
| assertNotNull(resp); | ||
| } | ||
| } | ||
|
|
||
| private void createViewOnCluster(String clusterAlias, String viewName, String query) { | ||
| assertAcked( | ||
| client(clusterAlias).execute( | ||
| PutViewAction.INSTANCE, | ||
| new PutViewAction.Request(TimeValue.THIRTY_SECONDS, TimeValue.THIRTY_SECONDS, new View(viewName, query)) | ||
| ).actionGet(30, java.util.concurrent.TimeUnit.SECONDS) | ||
| ); | ||
| } | ||
| } |
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.
Needed for creating a good error message.