From dc195f4db7f8bd09e0782140f0bf31108103a0da Mon Sep 17 00:00:00 2001 From: Luigi Dell'Aquila Date: Fri, 24 Jan 2025 12:59:23 +0100 Subject: [PATCH 01/23] ES|QL: Fix queries with document level security on lookup indexes (#120617) --- docs/changelog/120617.yaml | 5 + x-pack/plugin/build.gradle | 6 + .../xpack/esql/EsqlSecurityIT.java | 149 +++++++++++++++++- .../src/javaRestTest/resources/roles.yml | 47 ++++++ .../xpack/esql/action/EsqlCapabilities.java | 7 +- .../xpack/esql/analysis/Analyzer.java | 30 ---- .../esql/enrich/AbstractLookupService.java | 136 ++++------------ .../esql/enrich/EnrichLookupService.java | 89 ++++++++++- .../esql/enrich/LookupFromIndexService.java | 5 - .../esql/plan/logical/join/LookupJoin.java | 34 +++- .../test/esql/190_lookup_join.yml | 11 ++ .../esql/191_lookup_join_on_datastreams.yml | 68 ++++++++ 12 files changed, 437 insertions(+), 150 deletions(-) create mode 100644 docs/changelog/120617.yaml create mode 100644 x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml diff --git a/docs/changelog/120617.yaml b/docs/changelog/120617.yaml new file mode 100644 index 0000000000000..cdf93ef4e71f2 --- /dev/null +++ b/docs/changelog/120617.yaml @@ -0,0 +1,5 @@ +pr: 120617 +summary: Fix queries with document level security on lookup indexes +area: ES|QL +type: bug +issues: [120509] diff --git a/x-pack/plugin/build.gradle b/x-pack/plugin/build.gradle index 5ab0112d822ce..5987f75f4f198 100644 --- a/x-pack/plugin/build.gradle +++ b/x-pack/plugin/build.gradle @@ -96,5 +96,11 @@ tasks.named("yamlRestCompatTestTransform").configure({ task -> task.skipTest("esql/180_match_operator/match with non text field", "Match operator can now be used on non-text fields") task.skipTest("esql/180_match_operator/match with functions", "Error message changed") task.skipTest("esql/40_unsupported_types/semantic_text declared in mapping", "The semantic text field format changed") + task.skipTest("esql/190_lookup_join/Alias as lookup index", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-repeated-alias", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-repeated-index", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-pattern-multiple", "LOOKUP JOIN does not support index aliases for now") + task.skipTest("esql/190_lookup_join/alias-pattern-single", "LOOKUP JOIN does not support index aliases for now") + }) diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java index 5adac8fdd70d0..a809bd50a45b8 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/java/org/elasticsearch/xpack/esql/EsqlSecurityIT.java @@ -57,6 +57,10 @@ public class EsqlSecurityIT extends ESRestTestCase { .user("user4", "x-pack-test-password", "user4", false) .user("user5", "x-pack-test-password", "user5", false) .user("fls_user", "x-pack-test-password", "fls_user", false) + .user("fls_user2", "x-pack-test-password", "fls_user2", false) + .user("fls_user3", "x-pack-test-password", "fls_user3", false) + .user("fls_user4_1", "x-pack-test-password", "fls_user4_1", false) + .user("dls_user", "x-pack-test-password", "dls_user", false) .user("metadata1_read2", "x-pack-test-password", "metadata1_read2", false) .user("alias_user1", "x-pack-test-password", "alias_user1", false) .user("alias_user2", "x-pack-test-password", "alias_user2", false) @@ -92,7 +96,7 @@ private void indexDocument(String index, int id, double value, String org) throw public void indexDocuments() throws IOException { Settings lookupSettings = Settings.builder().put("index.mode", "lookup").build(); String mapping = """ - "properties":{"value": {"type": "double"}, "org": {"type": "keyword"}} + "properties":{"value": {"type": "double"}, "org": {"type": "keyword"}, "other": {"type": "keyword"}} """; createIndex("index", Settings.EMPTY, mapping); @@ -163,6 +167,32 @@ public void indexDocuments() throws IOException { """); assertOK(client().performRequest(aliasRequest)); } + + createMultiRoleUsers(); + } + + private void createMultiRoleUsers() throws IOException { + Request request = new Request("POST", "_security/user/dls_user2"); + request.setJsonEntity(""" + { + "password" : "x-pack-test-password", + "roles" : [ "dls_user", "dls_user2" ], + "full_name" : "Test Role", + "email" : "test.role@example.com" + } + """); + assertOK(client().performRequest(request)); + + request = new Request("POST", "_security/user/fls_user4"); + request.setJsonEntity(""" + { + "password" : "x-pack-test-password", + "roles" : [ "fls_user4_1", "fls_user4_2" ], + "full_name" : "Test Role", + "email" : "test.role@example.com" + } + """); + assertOK(client().performRequest(request)); } protected MapMatcher responseMatcher(Map result) { @@ -553,25 +583,130 @@ public void testLookupJoinIndexAllowed() throws Exception { ); assertThat(respMap.get("values"), equalTo(List.of(List.of(40.0, "sales")))); - // Alias, should find the index and the row - resp = runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org"); + // Aliases are not allowed in LOOKUP JOIN + var resp2 = expectThrows( + ResponseException.class, + () -> runESQLCommand("alias_user1", "ROW x = 31.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org") + ); + + assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]")); + assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + + // Aliases are not allowed in LOOKUP JOIN, regardless of alias filters + resp2 = expectThrows( + ResponseException.class, + () -> runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org") + ); + assertThat(resp2.getMessage(), containsString("Aliases and index patterns are not allowed for LOOKUP JOIN [lookup-first-alias]")); + assertThat(resp2.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); + } + + @SuppressWarnings("unchecked") + public void testLookupJoinDocLevelSecurity() throws Exception { + assumeTrue( + "Requires LOOKUP JOIN capability", + EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName())) + ); + + Response resp = runESQLCommand("dls_user", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) + ); + + assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, null)))); + + resp = runESQLCommand("dls_user", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) + ); + assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing")))); + + // same, but with a user that has two dls roles that allow him more visibility + + resp = runESQLCommand("dls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); assertOK(resp); respMap = entityAsMap(resp); assertThat( respMap.get("columns"), equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) ); - assertThat(respMap.get("values"), equalTo(List.of(List.of(31.0, "sales")))); - // Alias, for a row that's filtered out - resp = runESQLCommand("alias_user1", "ROW x = 123.0 | EVAL value = x | LOOKUP JOIN lookup-first-alias ON value | KEEP x, org"); + assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(40.0, "sales")))); + + resp = runESQLCommand("dls_user2", "ROW x = 32.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value | KEEP x, org"); assertOK(resp); respMap = entityAsMap(resp); assertThat( respMap.get("columns"), equalTo(List.of(Map.of("name", "x", "type", "double"), Map.of("name", "org", "type", "keyword"))) ); - assertThat(respMap.get("values"), equalTo(List.of(Arrays.asList(123.0, null)))); + assertThat(respMap.get("values"), equalTo(List.of(List.of(32.0, "marketing")))); + + } + + @SuppressWarnings("unchecked") + public void testLookupJoinFieldLevelSecurity() throws Exception { + assumeTrue( + "Requires LOOKUP JOIN capability", + EsqlSpecTestCase.hasCapabilities(adminClient(), List.of(EsqlCapabilities.Cap.JOIN_LOOKUP_V12.capabilityName())) + ); + + Response resp = runESQLCommand("fls_user2", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + Map respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword") + ) + ) + ); + + resp = runESQLCommand("fls_user3", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword"), + Map.of("name", "other", "type", "keyword") + ) + ) + + ); + + resp = runESQLCommand("fls_user4", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value"); + assertOK(resp); + respMap = entityAsMap(resp); + assertThat( + respMap.get("columns"), + equalTo( + List.of( + Map.of("name", "x", "type", "double"), + Map.of("name", "value", "type", "double"), + Map.of("name", "org", "type", "keyword") + ) + ) + ); + + ResponseException error = expectThrows( + ResponseException.class, + () -> runESQLCommand("fls_user4_1", "ROW x = 40.0 | EVAL value = x | LOOKUP JOIN lookup-user2 ON value") + ); + assertThat(error.getMessage(), containsString("Unknown column [value] in right side of join")); + assertThat(error.getResponse().getStatusLine().getStatusCode(), equalTo(HttpStatus.SC_BAD_REQUEST)); } public void testLookupJoinIndexForbidden() throws Exception { diff --git a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml index f46e7ef56f3a1..745ae43cf640c 100644 --- a/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml +++ b/x-pack/plugin/esql/qa/security/src/javaRestTest/resources/roles.yml @@ -93,6 +93,53 @@ fls_user: field_security: grant: [ value ] +fls_user2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org", "value" ] + +fls_user3: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org", "value", "other" ] + +fls_user4_1: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "org" ] + +fls_user4_2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + field_security: + grant: [ "value" ] + +dls_user: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + query: '{"match": {"org": "marketing"}}' + +dls_user2: + cluster: [] + indices: + - names: [ 'lookup-user2' ] + privileges: [ 'read' ] + query: '{"match": {"org": "sales"}}' + + logs_foo_all: cluster: [] indices: diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java index 12a25c9ce2453..182328b54c4c5 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/action/EsqlCapabilities.java @@ -751,7 +751,12 @@ public enum Cap { /** * Support named argument for function in map format. */ - OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot()); + OPTIONAL_NAMED_ARGUMENT_MAP_FOR_FUNCTION(Build.current().isSnapshot()), + + /** + * Disabled support for index aliases in lookup joins + */ + LOOKUP_JOIN_NO_ALIASES(JOIN_LOOKUP_V12.isEnabled()); private final boolean enabled; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index fc1b7f6329ab3..552e90e0e90f9 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -237,36 +237,6 @@ private LogicalPlan resolveIndex(UnresolvedRelation plan, IndexResolution indexR EsIndex esIndex = indexResolution.get(); - if (plan.indexMode().equals(IndexMode.LOOKUP)) { - String indexResolutionMessage = null; - - var indexNameWithModes = esIndex.indexNameWithModes(); - if (indexNameWithModes.size() != 1) { - indexResolutionMessage = "invalid [" - + table - + "] resolution in lookup mode to [" - + indexNameWithModes.size() - + "] indices"; - } else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) { - indexResolutionMessage = "invalid [" - + table - + "] resolution in lookup mode to an index in [" - + indexNameWithModes.values().iterator().next() - + "] mode"; - } - - if (indexResolutionMessage != null) { - return new UnresolvedRelation( - plan.source(), - plan.table(), - plan.frozen(), - plan.metadataFields(), - plan.indexMode(), - indexResolutionMessage, - plan.commandName() - ); - } - } var attributes = mappingAsAttributes(plan.source(), esIndex.mapping()); attributes.addAll(plan.metadataFields()); return new EsRelation( diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java index 961d74794961f..cb2582db2ad33 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/AbstractLookupService.java @@ -12,7 +12,6 @@ import org.elasticsearch.action.IndicesRequest; import org.elasticsearch.action.UnavailableShardsException; import org.elasticsearch.action.support.ChannelActionListener; -import org.elasticsearch.action.support.ContextPreservingActionListener; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.cluster.node.DiscoveryNode; @@ -23,7 +22,6 @@ import org.elasticsearch.common.CheckedBiFunction; import org.elasticsearch.common.collect.Iterators; import org.elasticsearch.common.io.stream.StreamInput; -import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.compute.data.Block; @@ -67,15 +65,6 @@ import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportResponse; import org.elasticsearch.transport.TransportService; -import org.elasticsearch.xpack.core.ClientHelper; -import org.elasticsearch.xpack.core.XPackSettings; -import org.elasticsearch.xpack.core.security.SecurityContext; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; -import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; -import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.support.Exceptions; -import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.core.expression.Alias; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; @@ -93,7 +82,6 @@ import java.util.Objects; import java.util.concurrent.Executor; import java.util.function.Function; -import java.util.stream.Collectors; import java.util.stream.IntStream; /** @@ -132,10 +120,10 @@ */ public abstract class AbstractLookupService { private final String actionName; - private final ClusterService clusterService; + protected final ClusterService clusterService; private final LookupShardContextFactory lookupShardContextFactory; - private final TransportService transportService; - private final Executor executor; + protected final TransportService transportService; + protected final Executor executor; private final BigArrays bigArrays; private final BlockFactory blockFactory; private final LocalCircuitBreaker.SizeSettings localBreakerSettings; @@ -218,97 +206,43 @@ protected static QueryList termQueryList( * Perform the actual lookup. */ public final void lookupAsync(R request, CancellableTask parentTask, ActionListener> outListener) { - ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - ActionListener> listener = ContextPreservingActionListener.wrapPreservingContext(outListener, threadContext); - hasPrivilege(listener.delegateFailureAndWrap((delegate, ignored) -> { - ClusterState clusterState = clusterService.state(); - GroupShardsIterator shardIterators = clusterService.operationRouting() - .searchShards(clusterState, new String[] { request.index }, Map.of(), "_local"); - if (shardIterators.size() != 1) { - delegate.onFailure(new EsqlIllegalArgumentException("target index {} has more than one shard", request.index)); - return; - } - ShardIterator shardIt = shardIterators.get(0); - ShardRouting shardRouting = shardIt.nextOrNull(); - ShardId shardId = shardIt.shardId(); - if (shardRouting == null) { - delegate.onFailure(new UnavailableShardsException(shardId, "target index is not available")); - return; - } - DiscoveryNode targetNode = clusterState.nodes().get(shardRouting.currentNodeId()); - T transportRequest = transportRequest(request, shardId); - // TODO: handle retry and avoid forking for the local lookup - try (ThreadContext.StoredContext unused = threadContext.stashWithOrigin(ClientHelper.ENRICH_ORIGIN)) { - transportService.sendChildRequest( - targetNode, - actionName, - transportRequest, - parentTask, - TransportRequestOptions.EMPTY, - new ActionListenerResponseHandler<>( - delegate.map(LookupResponse::takePages), - in -> readLookupResponse(in, blockFactory), - executor - ) - ); - } - })); - } - - /** - * Get the privilege required to perform the lookup. - *

- * If null is returned, no privilege check will be performed. - *

- */ - @Nullable - protected abstract String getRequiredPrivilege(); - - private void hasPrivilege(ActionListener outListener) { - final Settings settings = clusterService.getSettings(); - String privilegeName = getRequiredPrivilege(); - if (privilegeName == null - || settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false - || XPackSettings.SECURITY_ENABLED.get(settings) == false) { - outListener.onResponse(null); + ClusterState clusterState = clusterService.state(); + GroupShardsIterator shardIterators = clusterService.operationRouting() + .searchShards(clusterState, new String[] { request.index }, Map.of(), "_local"); + if (shardIterators.size() != 1) { + outListener.onFailure(new EsqlIllegalArgumentException("target index {} has more than one shard", request.index)); return; } - final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); - final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext); - final User user = securityContext.getUser(); - if (user == null) { - outListener.onFailure(new IllegalStateException("missing or unable to read authentication info on request")); + ShardIterator shardIt = shardIterators.get(0); + ShardRouting shardRouting = shardIt.nextOrNull(); + ShardId shardId = shardIt.shardId(); + if (shardRouting == null) { + outListener.onFailure(new UnavailableShardsException(shardId, "target index is not available")); return; } - HasPrivilegesRequest request = new HasPrivilegesRequest(); - request.username(user.principal()); - request.clusterPrivileges(privilegeName); - request.indexPrivileges(new RoleDescriptor.IndicesPrivileges[0]); - request.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]); - ActionListener listener = outListener.delegateFailureAndWrap((l, resp) -> { - if (resp.isCompleteMatch()) { - l.onResponse(null); - return; - } - String detailed = resp.getClusterPrivileges() - .entrySet() - .stream() - .filter(e -> e.getValue() == false) - .map(e -> "privilege [" + e.getKey() + "] is missing") - .collect(Collectors.joining(", ")); - String message = "user [" - + user.principal() - + "] doesn't have " - + "sufficient privileges to perform enrich lookup: " - + detailed; - l.onFailure(Exceptions.authorizationError(message)); - }); - transportService.sendRequest( - transportService.getLocalNode(), - HasPrivilegesAction.NAME, - request, + DiscoveryNode targetNode = clusterState.nodes().get(shardRouting.currentNodeId()); + T transportRequest = transportRequest(request, shardId); + // TODO: handle retry and avoid forking for the local lookup + sendChildRequest(parentTask, outListener, targetNode, transportRequest); + } + + protected void sendChildRequest( + CancellableTask parentTask, + ActionListener> delegate, + DiscoveryNode targetNode, + T transportRequest + ) { + transportService.sendChildRequest( + targetNode, + actionName, + transportRequest, + parentTask, TransportRequestOptions.EMPTY, - new ActionListenerResponseHandler<>(listener, HasPrivilegesResponse::new, executor) + new ActionListenerResponseHandler<>( + delegate.map(LookupResponse::takePages), + in -> readLookupResponse(in, blockFactory), + executor + ) ); } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java index acb4206ad7af8..480b69ecd8e60 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/EnrichLookupService.java @@ -8,10 +8,16 @@ package org.elasticsearch.xpack.esql.enrich; import org.elasticsearch.TransportVersions; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.ActionListenerResponseHandler; +import org.elasticsearch.action.support.ContextPreservingActionListener; +import org.elasticsearch.cluster.node.DiscoveryNode; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.common.util.BigArrays; +import org.elasticsearch.common.util.concurrent.ThreadContext; import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BlockFactory; import org.elasticsearch.compute.data.BlockStreamInput; @@ -23,9 +29,20 @@ import org.elasticsearch.index.mapper.RangeType; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.index.shard.ShardId; +import org.elasticsearch.tasks.CancellableTask; import org.elasticsearch.tasks.TaskId; +import org.elasticsearch.transport.TransportRequestOptions; import org.elasticsearch.transport.TransportService; +import org.elasticsearch.xpack.core.ClientHelper; +import org.elasticsearch.xpack.core.XPackSettings; +import org.elasticsearch.xpack.core.security.SecurityContext; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesAction; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesRequest; +import org.elasticsearch.xpack.core.security.action.user.HasPrivilegesResponse; +import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; import org.elasticsearch.xpack.core.security.authz.privilege.ClusterPrivilegeResolver; +import org.elasticsearch.xpack.core.security.support.Exceptions; +import org.elasticsearch.xpack.core.security.user.User; import org.elasticsearch.xpack.esql.EsqlIllegalArgumentException; import org.elasticsearch.xpack.esql.action.EsqlQueryAction; import org.elasticsearch.xpack.esql.core.expression.NamedExpression; @@ -36,6 +53,7 @@ import java.io.IOException; import java.util.List; +import java.util.stream.Collectors; /** * {@link EnrichLookupService} performs enrich lookup for a given input page. @@ -90,11 +108,6 @@ protected QueryList queryList(TransportRequest request, SearchExecutionContext c }; } - @Override - protected String getRequiredPrivilege() { - return ClusterPrivilegeResolver.MONITOR_ENRICH.name(); - } - @Override protected LookupResponse createLookupResponse(List pages, BlockFactory blockFactory) throws IOException { if (pages.size() != 1) { @@ -270,4 +283,70 @@ protected void innerRelease() { } } } + + @Override + protected void sendChildRequest( + CancellableTask parentTask, + ActionListener> delegate, + DiscoveryNode targetNode, + TransportRequest transportRequest + ) { + ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); + ActionListener> listener = ContextPreservingActionListener.wrapPreservingContext(delegate, threadContext); + hasEnrichPrivilege(listener.delegateFailureAndWrap((l, ignored) -> { + // Since we just checked the needed privileges + // we can access the index regardless of the user/role that is executing the query + try (ThreadContext.StoredContext unused = threadContext.stashWithOrigin(ClientHelper.ENRICH_ORIGIN)) { + super.sendChildRequest(parentTask, l, targetNode, transportRequest); + } + })); + } + + protected void hasEnrichPrivilege(ActionListener outListener) { + final Settings settings = clusterService.getSettings(); + String privilegeName = ClusterPrivilegeResolver.MONITOR_ENRICH.name(); + if (privilegeName == null + || settings.hasValue(XPackSettings.SECURITY_ENABLED.getKey()) == false + || XPackSettings.SECURITY_ENABLED.get(settings) == false) { + outListener.onResponse(null); + return; + } + final ThreadContext threadContext = transportService.getThreadPool().getThreadContext(); + final SecurityContext securityContext = new SecurityContext(Settings.EMPTY, threadContext); + final User user = securityContext.getUser(); + if (user == null) { + outListener.onFailure(new IllegalStateException("missing or unable to read authentication info on request")); + return; + } + HasPrivilegesRequest request = new HasPrivilegesRequest(); + request.username(user.principal()); + request.clusterPrivileges(privilegeName); + request.indexPrivileges(new RoleDescriptor.IndicesPrivileges[0]); + request.applicationPrivileges(new RoleDescriptor.ApplicationResourcePrivileges[0]); + ActionListener listener = outListener.delegateFailureAndWrap((l, resp) -> { + if (resp.isCompleteMatch()) { + l.onResponse(null); + return; + } + String detailed = resp.getClusterPrivileges() + .entrySet() + .stream() + .filter(e -> e.getValue() == false) + .map(e -> "privilege [" + e.getKey() + "] is missing") + .collect(Collectors.joining(", ")); + String message = "user [" + + user.principal() + + "] doesn't have " + + "sufficient privileges to perform enrich lookup: " + + detailed; + l.onFailure(Exceptions.authorizationError(message)); + }); + transportService.sendRequest( + transportService.getLocalNode(), + HasPrivilegesAction.NAME, + request, + TransportRequestOptions.EMPTY, + new ActionListenerResponseHandler<>(listener, HasPrivilegesResponse::new, executor) + ); + } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java index 9bea212a56aa8..131d8ddfa5ccd 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/enrich/LookupFromIndexService.java @@ -90,11 +90,6 @@ protected AbstractLookupService.LookupResponse readLookupResponse(StreamInput in return new LookupResponse(in, blockFactory); } - @Override - protected String getRequiredPrivilege() { - return null; - } - public static class Request extends AbstractLookupService.Request { private final String matchField; diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java index 4e009156072df..c29cf0ec7f414 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/join/LookupJoin.java @@ -7,9 +7,13 @@ package org.elasticsearch.xpack.esql.plan.logical.join; +import org.elasticsearch.index.IndexMode; +import org.elasticsearch.xpack.esql.capabilities.PostAnalysisVerificationAware; +import org.elasticsearch.xpack.esql.common.Failures; import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; +import org.elasticsearch.xpack.esql.plan.logical.EsRelation; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.SurrogateLogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.UsingJoinType; @@ -17,12 +21,13 @@ import java.util.List; import static java.util.Collections.emptyList; +import static org.elasticsearch.xpack.esql.common.Failure.fail; import static org.elasticsearch.xpack.esql.plan.logical.join.JoinTypes.LEFT; /** * Lookup join - specialized LEFT (OUTER) JOIN between the main left side and a lookup index (index_mode = lookup) on the right. */ -public class LookupJoin extends Join implements SurrogateLogicalPlan { +public class LookupJoin extends Join implements SurrogateLogicalPlan, PostAnalysisVerificationAware { public LookupJoin(Source source, LogicalPlan left, LogicalPlan right, List joinFields) { this(source, left, right, new UsingJoinType(LEFT, joinFields), emptyList(), emptyList(), emptyList()); @@ -71,4 +76,31 @@ protected NodeInfo info() { config().rightFields() ); } + + @Override + public void postAnalysisVerification(Failures failures) { + super.postAnalysisVerification(failures); + right().forEachDown(EsRelation.class, esr -> { + var indexNameWithModes = esr.indexNameWithModes(); + if (indexNameWithModes.size() != 1) { + failures.add( + fail(esr, "invalid [{}] resolution in lookup mode to [{}] indices", esr.indexPattern(), indexNameWithModes.size()) + ); + } else if (indexNameWithModes.values().iterator().next() != IndexMode.LOOKUP) { + failures.add( + fail( + esr, + "invalid [{}] resolution in lookup mode to an index in [{}] mode", + esr.indexPattern(), + indexNameWithModes.values().iterator().next() + ) + ); + } + + // this check is crucial for security: ES|QL would use the concrete indices, so it would bypass the security on the alias + if (esr.concreteIndices().contains(esr.indexPattern()) == false) { + failures.add(fail(this, "Aliases and index patterns are not allowed for LOOKUP JOIN [{}]", esr.indexPattern())); + } + }); + } } diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml index e8c9df0d3287e..f72cdd65b275c 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/190_lookup_join.yml @@ -102,7 +102,10 @@ non-lookup index: - contains: { error.reason: "Found 1 problem\nline 1:45: invalid [test] resolution in lookup mode to an index in [standard] mode" } --- + "Alias as lookup index": + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -117,6 +120,8 @@ non-lookup index: --- alias-repeated-alias: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -131,6 +136,8 @@ alias-repeated-alias: --- alias-repeated-index: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -145,6 +152,8 @@ alias-repeated-index: --- alias-pattern-multiple: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: @@ -156,6 +165,8 @@ alias-pattern-multiple: --- alias-pattern-single: + - skip: + awaits_fix: "LOOKUP JOIN does not support index aliases for now" - do: esql.query: body: diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml new file mode 100644 index 0000000000000..6f9b70b0d94f1 --- /dev/null +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/esql/191_lookup_join_on_datastreams.yml @@ -0,0 +1,68 @@ +--- +setup: + - requires: + test_runner_features: [capabilities, contains] + capabilities: + - method: POST + path: /_query + parameters: [] + capabilities: [lookup_join_no_aliases] + reason: "uses LOOKUP JOIN" + + - do: + cluster.put_component_template: + name: my_settings + body: + template: + settings: + index: + mode: lookup + + + - do: + cluster.put_component_template: + name: my_mappings + body: + template: + mappings: + properties: + "@timestamp": + type: date + x: + type: keyword + + - do: + indices.put_index_template: + name: my_index_template + body: + index_patterns: my_data_stream* + data_stream: {} + composed_of: [ "my_mappings", "my_settings" ] + priority: 500 + + + - do: + bulk: + index: "my_data_stream" + refresh: true + body: + - { "index": { } } + - { "x": "foo", "y": "y1" } + - { "index": { } } + - { "x": "bar", "y": "y2" } + + + +--- +"data streams not supported in LOOKUP JOIN": + - do: + esql.query: + body: + query: 'row x = "foo" | LOOKUP JOIN my_data_stream ON x' + catch: "bad_request" + + - match: { error.type: "verification_exception" } + - contains: { error.reason: "Found 1 problem\nline 1:17: Aliases and index patterns are not allowed for LOOKUP JOIN [my_data_stream]" } + + + From 385e1fdf21fb30c20adf94c29f35703b344b97f5 Mon Sep 17 00:00:00 2001 From: Navarone Feekery <13634519+navarone-feekery@users.noreply.github.com> Date: Fri, 24 Jan 2025 13:10:38 +0100 Subject: [PATCH 02/23] [Search] Add system index descriptors to Connector indices (#118991) Update the .elastic-connectors and .elastic-connectors-sync-jobs indices into system indices --- .../xpack/core/ClientHelper.java | 3 + ...json => elastic-connectors-sync-jobs.json} | 30 ++-- ...-mappings.json => elastic-connectors.json} | 51 ++++--- .../elastic-connectors-settings.json | 14 -- .../elastic-connectors-sync-jobs.json | 14 -- .../connector/elastic-connectors.json | 14 -- .../xpack/application/EnterpriseSearch.java | 9 +- .../connector/ConnectorIndexService.java | 50 +++++- .../connector/ConnectorTemplateRegistry.java | 81 +--------- .../syncjob/ConnectorSyncJobIndexService.java | 47 +++++- .../connector/ConnectorIndexServiceTests.java | 25 ++- .../ConnectorTemplateRegistryTests.java | 144 +----------------- .../connector/ConnectorTestUtils.java | 53 +------ .../ConnectorSyncJobIndexServiceTests.java | 44 ++++-- .../syncjob/ConnectorSyncJobTestUtils.java | 26 ++-- 15 files changed, 230 insertions(+), 375 deletions(-) rename x-pack/plugin/core/template-resources/src/main/resources/{entsearch/connector/elastic-connectors-sync-jobs-mappings.json => elastic-connectors-sync-jobs.json} (88%) rename x-pack/plugin/core/template-resources/src/main/resources/{entsearch/connector/elastic-connectors-mappings.json => elastic-connectors.json} (92%) delete mode 100644 x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-settings.json delete mode 100644 x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs.json delete mode 100644 x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors.json diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java index 9a0d1a58a30a1..680b72cb970c9 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/ClientHelper.java @@ -196,6 +196,9 @@ private static String maybeRewriteSingleAuthenticationHeaderForVersion( public static final String APM_ORIGIN = "apm"; public static final String OTEL_ORIGIN = "otel"; public static final String REINDEX_DATA_STREAM_ORIGIN = "reindex_data_stream"; + // TODO consolidate the Kibana origin with the one defined in org/elasticsearch/kibana/KibanaPlugin.java + public static final String KIBANA_ORIGIN = "kibana"; + public static final String CLOUD_ORIGIN = "cloud"; private ClientHelper() {} diff --git a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs-mappings.json b/x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors-sync-jobs.json similarity index 88% rename from x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs-mappings.json rename to x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors-sync-jobs.json index 4dd6e0681c7cc..7d1e7fa3a0418 100644 --- a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs-mappings.json +++ b/x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors-sync-jobs.json @@ -1,12 +1,16 @@ { - "template": { - "aliases": { - ".elastic-connectors-sync-jobs": {} - }, - "mappings": { - "dynamic": "false", + "settings": { + "index": { + "number_of_shards": "1", + "auto_expand_replicas": "0-1" + } + }, + "mappings": { + "_doc": { + "dynamic": "strict", "_meta": { - "version": ${xpack.application.connector.template.version} + "version": "${elastic-connectors-sync-jobs.version}", + "managed_index_mappings_version": ${elastic-connectors-sync-jobs.managed.index.version} }, "properties": { "cancelation_requested_at": { @@ -21,9 +25,11 @@ "connector": { "properties": { "configuration": { + "dynamic": "false", "type": "object" }, "filtering": { + "dynamic": "false", "properties": { "advanced_snippet": { "properties": { @@ -91,6 +97,7 @@ "type": "keyword" }, "pipeline": { + "dynamic": "false", "properties": { "extract_binary_content": { "type": "boolean" @@ -110,6 +117,7 @@ "type": "keyword" }, "sync_cursor": { + "dynamic": "false", "type": "object" } } @@ -136,6 +144,7 @@ "type": "date" }, "metadata": { + "dynamic": "false", "type": "object" }, "started_at": { @@ -155,10 +164,5 @@ } } } - }, - "_meta": { - "description": "Built-in mappings applied by default to elastic-connectors indices", - "managed": true - }, - "version": ${xpack.application.connector.template.version} + } } diff --git a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-mappings.json b/x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors.json similarity index 92% rename from x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-mappings.json rename to x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors.json index 25409dbf8460e..a98018e76f0e0 100644 --- a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-mappings.json +++ b/x-pack/plugin/core/template-resources/src/main/resources/elastic-connectors.json @@ -1,29 +1,35 @@ { - "template": { - "aliases": { - ".elastic-connectors": {} - }, - "mappings": { - "dynamic": "false", + "settings": { + "index": { + "number_of_shards": "1", + "auto_expand_replicas": "0-1" + } + }, + "mappings": { + "_doc": { + "dynamic": "strict", "_meta": { - "pipeline": { - "default_name": "search-default-ingestion", - "default_extract_binary_content": true, - "default_run_ml_inference": true, - "default_reduce_whitespace": true - }, - "version": ${xpack.application.connector.template.version} + "version": "${elastic-connectors.version}", + "managed_index_mappings_version": ${elastic-connectors.managed.index.version} }, "properties": { "api_key_id": { "type": "keyword" }, + "api_key_secret_id": { + "type": "keyword" + }, "configuration": { + "dynamic": "false", "type": "object" }, "custom_scheduling": { + "dynamic": "false", "type": "object" }, + "deleted": { + "type": "boolean" + }, "description": { "type": "text" }, @@ -31,6 +37,7 @@ "type": "keyword" }, "features": { + "dynamic": "false", "properties": { "filtering_advanced_config": { "type": "boolean" @@ -66,6 +73,7 @@ } }, "filtering": { + "dynamic": "false", "properties": { "active": { "properties": { @@ -78,6 +86,7 @@ "type": "date" }, "value": { + "dynamic": "false", "type": "object" } } @@ -143,6 +152,7 @@ "type": "date" }, "value": { + "dynamic": "false", "type": "object" } } @@ -242,6 +252,7 @@ "type": "keyword" }, "pipeline": { + "dynamic": "false", "properties": { "extract_binary_content": { "type": "boolean" @@ -258,6 +269,7 @@ } }, "scheduling": { + "dynamic": "false", "properties": { "access_control": { "properties": { @@ -298,22 +310,13 @@ "type": "keyword" }, "sync_cursor": { + "dynamic": "false", "type": "object" }, "sync_now": { "type": "boolean" - }, - "deleted": { - "type": "boolean" } } } - }, - "_meta": { - "description": "Built-in mappings applied by default to elastic-connectors indices", - "managed": true - }, - "version": ${xpack.application.connector.template.version} + } } - - diff --git a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-settings.json b/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-settings.json deleted file mode 100644 index 6ff9510574281..0000000000000 --- a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-settings.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "template": { - "settings": { - "hidden": true, - "number_of_shards": "1", - "auto_expand_replicas": "0-1" - } - }, - "_meta": { - "description": "Built-in settings applied by default to connector management indices", - "managed": true - }, - "version": ${xpack.application.connector.template.version} -} diff --git a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs.json b/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs.json deleted file mode 100644 index db5404a30c6e4..0000000000000 --- a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors-sync-jobs.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "index_patterns": ["${connectors-sync-jobs.index_pattern}"], - "priority": 100, - "composed_of": [ - "elastic-connectors-settings", - "elastic-connectors-sync-jobs-mappings" - ], - "allow_auto_create": true, - "_meta": { - "description": "Built-in template for elastic-connectors-sync-jobs", - "managed": true - }, - "version": ${xpack.application.connector.template.version} -} diff --git a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors.json b/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors.json deleted file mode 100644 index 17c0b1eef0610..0000000000000 --- a/x-pack/plugin/core/template-resources/src/main/resources/entsearch/connector/elastic-connectors.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "index_patterns": ["${connectors.index_pattern}"], - "priority": 100, - "composed_of": [ - "elastic-connectors-settings", - "elastic-connectors-mappings" - ], - "allow_auto_create": true, - "_meta": { - "description": "Built-in template for elastic-connectors", - "managed": true - }, - "version": ${xpack.application.connector.template.version} -} diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearch.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearch.java index df1c76ccf770f..4142d907d0c5c 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearch.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/EnterpriseSearch.java @@ -46,6 +46,7 @@ import org.elasticsearch.xpack.application.analytics.action.TransportPutAnalyticsCollectionAction; import org.elasticsearch.xpack.application.analytics.ingest.AnalyticsEventIngestConfig; import org.elasticsearch.xpack.application.connector.ConnectorAPIFeature; +import org.elasticsearch.xpack.application.connector.ConnectorIndexService; import org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry; import org.elasticsearch.xpack.application.connector.action.DeleteConnectorAction; import org.elasticsearch.xpack.application.connector.action.GetConnectorAction; @@ -124,6 +125,7 @@ import org.elasticsearch.xpack.application.connector.secrets.action.TransportGetConnectorSecretAction; import org.elasticsearch.xpack.application.connector.secrets.action.TransportPostConnectorSecretAction; import org.elasticsearch.xpack.application.connector.secrets.action.TransportPutConnectorSecretAction; +import org.elasticsearch.xpack.application.connector.syncjob.ConnectorSyncJobIndexService; import org.elasticsearch.xpack.application.connector.syncjob.action.CancelConnectorSyncJobAction; import org.elasticsearch.xpack.application.connector.syncjob.action.CheckInConnectorSyncJobAction; import org.elasticsearch.xpack.application.connector.syncjob.action.ClaimConnectorSyncJobAction; @@ -477,7 +479,12 @@ public Collection createComponents(PluginServices services) { @Override public Collection getSystemIndexDescriptors(Settings settings) { Collection systemIndices = new ArrayList<>( - List.of(SearchApplicationIndexService.getSystemIndexDescriptor(), QueryRulesIndexService.getSystemIndexDescriptor()) + List.of( + SearchApplicationIndexService.getSystemIndexDescriptor(), + QueryRulesIndexService.getSystemIndexDescriptor(), + ConnectorSyncJobIndexService.getSystemIndexDescriptor(), + ConnectorIndexService.getSystemIndexDescriptor() + ) ); if (ConnectorSecretsFeature.isEnabled()) { diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java index 3120124c17523..a9ca8552feeea 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorIndexService.java @@ -10,10 +10,12 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.get.GetRequest; import org.elasticsearch.action.index.IndexRequest; @@ -33,6 +35,7 @@ import org.elasticsearch.index.query.TermQueryBuilder; import org.elasticsearch.index.query.TermsQueryBuilder; import org.elasticsearch.index.query.WildcardQueryBuilder; +import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.Script; import org.elasticsearch.script.ScriptType; @@ -59,6 +62,7 @@ import org.elasticsearch.xpack.application.connector.filtering.FilteringValidationState; import org.elasticsearch.xpack.application.connector.syncjob.ConnectorSyncJob; import org.elasticsearch.xpack.application.connector.syncjob.ConnectorSyncJobIndexService; +import org.elasticsearch.xpack.core.template.TemplateUtils; import java.time.Instant; import java.util.ArrayList; @@ -76,6 +80,7 @@ import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.application.connector.ConnectorFiltering.fromXContentBytesConnectorFiltering; import static org.elasticsearch.xpack.application.connector.ConnectorFiltering.sortFilteringRulesByOrder; +import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTORS_ALLOWED_PRODUCT_ORIGINS; import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.MANAGED_CONNECTOR_INDEX_PREFIX; import static org.elasticsearch.xpack.core.ClientHelper.CONNECTORS_ORIGIN; @@ -87,7 +92,20 @@ public class ConnectorIndexService { // The client to interact with the system index (internal user). private final Client clientWithOrigin; - public static final String CONNECTOR_INDEX_NAME = ConnectorTemplateRegistry.CONNECTOR_INDEX_NAME_PATTERN; + // TODO use proper version IDs (see org/elasticsearch/xpack/application/rules/QueryRulesIndexService.java) + // TODO if this version is updated, a test should be added to + // javaRestTest/java/org/elasticsearch/xpack/application/FullClusterRestartIT.java + private static final int CONNECTORS_INDEX_VERSION = 1; + // TODO rename to CONNECTOR_ALIAS_NAME + public static final String CONNECTOR_INDEX_NAME = ".elastic-connectors"; + public static final String CONNECTOR_INDEX_PREFIX = ".elastic-connectors-v"; + public static final String CONNECTOR_CONCRETE_INDEX_NAME = CONNECTOR_INDEX_PREFIX + CONNECTORS_INDEX_VERSION; + // The index pattern needs a stricter regex to prevent conflicts with .elastic-connectors-sync-jobs + + public static final String CONNECTOR_INDEX_NAME_PATTERN = CONNECTOR_INDEX_PREFIX + "*"; + + private static final String CONNECTORS_MAPPING_VERSION_VARIABLE = "elastic-connectors.version"; + private static final String CONNECTORS_MAPPING_MANAGED_VERSION_VARIABLE = "elastic-connectors.managed.index.version"; /** * @param client A client for executing actions on the connector index @@ -96,6 +114,36 @@ public ConnectorIndexService(Client client) { this.clientWithOrigin = new OriginSettingClient(client, CONNECTORS_ORIGIN); } + /** + * Returns the {@link SystemIndexDescriptor} for the Connector system index. + * + * @return The {@link SystemIndexDescriptor} for the Connector system index. + */ + public static SystemIndexDescriptor getSystemIndexDescriptor() { + PutIndexTemplateRequest request = new PutIndexTemplateRequest(); + String templateSource = TemplateUtils.loadTemplate( + "/elastic-connectors.json", + Version.CURRENT.toString(), + CONNECTORS_MAPPING_VERSION_VARIABLE, + Map.of(CONNECTORS_MAPPING_MANAGED_VERSION_VARIABLE, Integer.toString(CONNECTORS_INDEX_VERSION)) + ); + request.source(templateSource, XContentType.JSON); + + // The index pattern needs a stricter regex to prevent conflicts with .elastic-connectors-sync-jobs + return SystemIndexDescriptor.builder() + .setIndexPattern(CONNECTOR_INDEX_NAME_PATTERN) + .setPrimaryIndex(CONNECTOR_CONCRETE_INDEX_NAME) + .setAliasName(CONNECTOR_INDEX_NAME) + .setDescription("Search connectors") + .setMappings(request.mappings()) + .setSettings(request.settings()) + .setOrigin(CONNECTORS_ORIGIN) + .setType(SystemIndexDescriptor.Type.EXTERNAL_MANAGED) + .setAllowedElasticProductOrigins(CONNECTORS_ALLOWED_PRODUCT_ORIGINS) + .setNetNew() + .build(); + } + /** * Creates or updates the {@link Connector} in the underlying index with a specific doc ID * if connectorId is provided. Otherwise, the connector doc is indexed with auto-generated doc ID. diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistry.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistry.java index fd35acc89db5c..97ac05c443ad0 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistry.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistry.java @@ -8,25 +8,23 @@ package org.elasticsearch.xpack.application.connector; import org.elasticsearch.client.internal.Client; -import org.elasticsearch.cluster.metadata.ComponentTemplate; import org.elasticsearch.cluster.metadata.ComposableIndexTemplate; import org.elasticsearch.cluster.service.ClusterService; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.threadpool.ThreadPool; import org.elasticsearch.xcontent.NamedXContentRegistry; -import org.elasticsearch.xcontent.XContentParserConfiguration; -import org.elasticsearch.xcontent.json.JsonXContent; import org.elasticsearch.xpack.core.template.IndexTemplateConfig; import org.elasticsearch.xpack.core.template.IndexTemplateRegistry; import org.elasticsearch.xpack.core.template.IngestPipelineConfig; import org.elasticsearch.xpack.core.template.JsonIngestPipelineConfig; -import java.io.IOException; -import java.util.HashMap; import java.util.List; import java.util.Map; +import static org.elasticsearch.xpack.core.ClientHelper.CLOUD_ORIGIN; +import static org.elasticsearch.xpack.core.ClientHelper.CONNECTORS_ORIGIN; import static org.elasticsearch.xpack.core.ClientHelper.ENT_SEARCH_ORIGIN; +import static org.elasticsearch.xpack.core.ClientHelper.KIBANA_ORIGIN; public class ConnectorTemplateRegistry extends IndexTemplateRegistry { @@ -34,13 +32,6 @@ public class ConnectorTemplateRegistry extends IndexTemplateRegistry { static final int REGISTRY_VERSION = 3; // Connector indices constants - - public static final String CONNECTOR_INDEX_NAME_PATTERN = ".elastic-connectors-v1"; - public static final String CONNECTOR_TEMPLATE_NAME = "elastic-connectors"; - - public static final String CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN = ".elastic-connectors-sync-jobs-v1"; - public static final String CONNECTOR_SYNC_JOBS_TEMPLATE_NAME = "elastic-connectors-sync-jobs"; - public static final String ACCESS_CONTROL_INDEX_PREFIX = ".search-acl-filter-"; public static final String ACCESS_CONTROL_INDEX_NAME_PATTERN = ".search-acl-filter-*"; public static final String ACCESS_CONTROL_TEMPLATE_NAME = "search-acl-filter"; @@ -58,51 +49,8 @@ public class ConnectorTemplateRegistry extends IndexTemplateRegistry { // Variable used to replace template version in index templates public static final String TEMPLATE_VERSION_VARIABLE = "xpack.application.connector.template.version"; - private static final String MAPPINGS_SUFFIX = "-mappings"; - - private static final String SETTINGS_SUFFIX = "-settings"; - - private static final String JSON_EXTENSION = ".json"; - - static final Map COMPONENT_TEMPLATES; - - static { - final Map componentTemplates = new HashMap<>(); - for (IndexTemplateConfig config : List.of( - new IndexTemplateConfig( - CONNECTOR_TEMPLATE_NAME + MAPPINGS_SUFFIX, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_TEMPLATE_NAME + MAPPINGS_SUFFIX + JSON_EXTENSION, - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE - ), - new IndexTemplateConfig( - CONNECTOR_TEMPLATE_NAME + SETTINGS_SUFFIX, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_TEMPLATE_NAME + SETTINGS_SUFFIX + JSON_EXTENSION, - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE - ), - new IndexTemplateConfig( - CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + MAPPINGS_SUFFIX, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + MAPPINGS_SUFFIX + JSON_EXTENSION, - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE - ), - new IndexTemplateConfig( - CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + SETTINGS_SUFFIX, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_TEMPLATE_NAME + SETTINGS_SUFFIX + JSON_EXTENSION, - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE - ) - )) { - - try (var parser = JsonXContent.jsonXContent.createParser(XContentParserConfiguration.EMPTY, config.loadBytes())) { - componentTemplates.put(config.getTemplateName(), ComponentTemplate.parse(parser)); - } catch (IOException e) { - throw new AssertionError(e); - } - } - COMPONENT_TEMPLATES = Map.copyOf(componentTemplates); - } + // Sources allowed to access system indices using X-elastic-product-origin header + public static final List CONNECTORS_ALLOWED_PRODUCT_ORIGINS = List.of(KIBANA_ORIGIN, CONNECTORS_ORIGIN, CLOUD_ORIGIN); @Override protected List getIngestPipelines() { @@ -117,20 +65,6 @@ protected List getIngestPipelines() { } static final Map COMPOSABLE_INDEX_TEMPLATES = parseComposableTemplates( - new IndexTemplateConfig( - CONNECTOR_TEMPLATE_NAME, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_TEMPLATE_NAME + ".json", - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE, - Map.of("connectors.index_pattern", CONNECTOR_INDEX_NAME_PATTERN) - ), - new IndexTemplateConfig( - CONNECTOR_SYNC_JOBS_TEMPLATE_NAME, - ROOT_TEMPLATE_RESOURCE_PATH + CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + ".json", - REGISTRY_VERSION, - TEMPLATE_VERSION_VARIABLE, - Map.of("connectors-sync-jobs.index_pattern", CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN) - ), new IndexTemplateConfig( ACCESS_CONTROL_TEMPLATE_NAME, ROOT_TEMPLATE_RESOURCE_PATH + ACCESS_CONTROL_TEMPLATE_NAME + ".json", @@ -154,11 +88,6 @@ protected String getOrigin() { return ENT_SEARCH_ORIGIN; } - @Override - protected Map getComponentTemplateConfigs() { - return COMPONENT_TEMPLATES; - } - @Override protected Map getComposableTemplateConfigs() { return COMPOSABLE_INDEX_TEMPLATES; diff --git a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexService.java b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexService.java index f46d915a7123f..85de2f900ddff 100644 --- a/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexService.java +++ b/x-pack/plugin/ent-search/src/main/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexService.java @@ -11,10 +11,12 @@ import org.elasticsearch.ElasticsearchStatusException; import org.elasticsearch.ExceptionsHelper; import org.elasticsearch.ResourceNotFoundException; +import org.elasticsearch.Version; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.admin.indices.template.put.PutIndexTemplateRequest; import org.elasticsearch.action.bulk.BulkItemResponse; import org.elasticsearch.action.delete.DeleteRequest; import org.elasticsearch.action.delete.DeleteResponse; @@ -40,6 +42,7 @@ import org.elasticsearch.index.reindex.BulkByScrollResponse; import org.elasticsearch.index.reindex.DeleteByQueryAction; import org.elasticsearch.index.reindex.DeleteByQueryRequest; +import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.SearchHit; import org.elasticsearch.search.builder.SearchSourceBuilder; @@ -49,10 +52,10 @@ import org.elasticsearch.xpack.application.connector.Connector; import org.elasticsearch.xpack.application.connector.ConnectorFiltering; import org.elasticsearch.xpack.application.connector.ConnectorSyncStatus; -import org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry; import org.elasticsearch.xpack.application.connector.filtering.FilteringRules; import org.elasticsearch.xpack.application.connector.syncjob.action.PostConnectorSyncJobAction; import org.elasticsearch.xpack.application.connector.syncjob.action.UpdateConnectorSyncJobIngestionStatsAction; +import org.elasticsearch.xpack.core.template.TemplateUtils; import java.io.IOException; import java.time.Instant; @@ -69,6 +72,7 @@ import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.application.connector.ConnectorIndexService.CONNECTOR_INDEX_NAME; +import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTORS_ALLOWED_PRODUCT_ORIGINS; import static org.elasticsearch.xpack.core.ClientHelper.CONNECTORS_ORIGIN; /** @@ -81,7 +85,17 @@ public class ConnectorSyncJobIndexService { // The client to interact with the system index (internal user). private final Client clientWithOrigin; - public static final String CONNECTOR_SYNC_JOB_INDEX_NAME = ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN; + // TODO use proper version IDs (see org/elasticsearch/xpack/application/rules/QueryRulesIndexService.java) + // TODO if this version is updated, a test should be added to + // javaRestTest/java/org/elasticsearch/xpack/application/FullClusterRestartIT.java + private static final int CONNECTOR_SYNC_JOB_INDEX_VERSION = 1; + public static final String CONNECTOR_SYNC_JOB_INDEX_NAME = ".elastic-connectors-sync-jobs"; + public static final String CONNECTOR_SYNC_JOB_INDEX_PREFIX = ".elastic-connectors-sync-jobs-v"; + public static final String CONNECTOR_SYNC_JOB_CONCRETE_INDEX_NAME = CONNECTOR_SYNC_JOB_INDEX_PREFIX + CONNECTOR_SYNC_JOB_INDEX_VERSION; + public static final String CONNECTOR_SYNC_JOB_INDEX_NAME_PATTERN = CONNECTOR_SYNC_JOB_INDEX_NAME + "*"; + + private static final String CONNECTOR_SYNC_JOB_MAPPING_VERSION_VARIABLE = "elastic-connectors-sync-jobs.version"; + private static final String CONNECTOR_SYNC_JOB_MAPPING_MANAGED_VERSION_VARIABLE = "elastic-connectors-sync-jobs.managed.index.version"; /** * @param client A client for executing actions on the connectors sync jobs index. @@ -90,6 +104,35 @@ public ConnectorSyncJobIndexService(Client client) { this.clientWithOrigin = new OriginSettingClient(client, CONNECTORS_ORIGIN); } + /** + * Returns the {@link SystemIndexDescriptor} for the Connector system index. + * + * @return The {@link SystemIndexDescriptor} for the Connector system index. + */ + public static SystemIndexDescriptor getSystemIndexDescriptor() { + PutIndexTemplateRequest request = new PutIndexTemplateRequest(); + String templateSource = TemplateUtils.loadTemplate( + "/elastic-connectors-sync-jobs.json", + Version.CURRENT.toString(), + CONNECTOR_SYNC_JOB_MAPPING_VERSION_VARIABLE, + Map.of(CONNECTOR_SYNC_JOB_MAPPING_MANAGED_VERSION_VARIABLE, Integer.toString(CONNECTOR_SYNC_JOB_INDEX_VERSION)) + ); + request.source(templateSource, XContentType.JSON); + + return SystemIndexDescriptor.builder() + .setIndexPattern(CONNECTOR_SYNC_JOB_INDEX_NAME_PATTERN) + .setPrimaryIndex(CONNECTOR_SYNC_JOB_CONCRETE_INDEX_NAME) + .setAliasName(CONNECTOR_SYNC_JOB_INDEX_NAME) + .setDescription("Search connectors sync jobs") + .setMappings(request.mappings()) + .setSettings(request.settings()) + .setOrigin(CONNECTORS_ORIGIN) + .setType(SystemIndexDescriptor.Type.EXTERNAL_MANAGED) + .setAllowedElasticProductOrigins(CONNECTORS_ALLOWED_PRODUCT_ORIGINS) + .setNetNew() + .build(); + } + /** * @param request Request for creating a connector sync job. * @param listener Listener to respond to a successful response or an error. diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java index 7b6d9c9b14df9..53a8c7ac96944 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorIndexServiceTests.java @@ -14,7 +14,9 @@ import org.elasticsearch.action.update.UpdateResponse; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.core.Tuple; +import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.script.MockScriptEngine; import org.elasticsearch.script.MockScriptPlugin; @@ -59,7 +61,6 @@ import static org.elasticsearch.xpack.application.connector.ConnectorTestUtils.getRandomConnectorFeatures; import static org.elasticsearch.xpack.application.connector.ConnectorTestUtils.getRandomCronExpression; import static org.elasticsearch.xpack.application.connector.ConnectorTestUtils.randomConnectorFeatureEnabled; -import static org.elasticsearch.xpack.application.connector.ConnectorTestUtils.registerSimplifiedConnectorIndexTemplates; import static org.hamcrest.CoreMatchers.anyOf; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.not; @@ -72,7 +73,6 @@ public class ConnectorIndexServiceTests extends ESSingleNodeTestCase { @Before public void setup() { - registerSimplifiedConnectorIndexTemplates(indicesAdmin()); this.connectorIndexService = new ConnectorIndexService(client()); } @@ -80,6 +80,7 @@ public void setup() { protected Collection> getPlugins() { List> plugins = new ArrayList<>(super.getPlugins()); plugins.add(MockPainlessScriptEngine.TestPlugin.class); + plugins.add(ConnectorIndexServiceTests.TestPlugin.class); return plugins; } @@ -1612,4 +1613,24 @@ public void execute() { } } + /** + * Test plugin to register the {@link ConnectorIndexService} system index descriptor. + */ + public static class TestPlugin extends Plugin implements SystemIndexPlugin { + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return List.of(ConnectorIndexService.getSystemIndexDescriptor()); + } + + @Override + public String getFeatureName() { + return this.getClass().getSimpleName(); + } + + @Override + public String getFeatureDescription() { + return this.getClass().getCanonicalName(); + } + } + } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistryTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistryTests.java index 068b99626af9d..89bdabe78300c 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistryTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTemplateRegistryTests.java @@ -55,15 +55,13 @@ import java.util.stream.Collectors; import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.ACCESS_CONTROL_INDEX_NAME_PATTERN; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_INDEX_NAME_PATTERN; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN; import static org.hamcrest.Matchers.contains; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.oneOf; import static org.hamcrest.Matchers.sameInstance; +import static org.junit.Assert.assertNotNull; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.spy; import static org.mockito.Mockito.when; @@ -92,14 +90,6 @@ public void testThatNonExistingComposableTemplatesAreAddedImmediately() throws E DiscoveryNode node = DiscoveryNodeUtils.create("node"); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); Map existingComponentTemplates = Map.of( - ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-mappings", - ConnectorTemplateRegistry.REGISTRY_VERSION, - ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-settings", - ConnectorTemplateRegistry.REGISTRY_VERSION, - ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-mappings", - ConnectorTemplateRegistry.REGISTRY_VERSION, - ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-settings", - ConnectorTemplateRegistry.REGISTRY_VERSION, ConnectorTemplateRegistry.ACCESS_CONTROL_TEMPLATE_NAME, ConnectorTemplateRegistry.REGISTRY_VERSION ); @@ -125,131 +115,6 @@ public void testThatNonExistingComposableTemplatesAreAddedImmediately() throws E }); } - public void testThatNonExistingComponentTemplatesAreAddedImmediately() throws Exception { - DiscoveryNode node = DiscoveryNodeUtils.create("node"); - DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); - - ClusterChangedEvent event = createClusterChangedEvent( - Collections.emptyMap(), - Collections.emptyMap(), - Collections.singletonMap(ConnectorTemplateRegistry.SEARCH_DEFAULT_PIPELINE_NAME, ConnectorTemplateRegistry.REGISTRY_VERSION), - Collections.emptyMap(), - nodes - ); - - AtomicInteger calledTimes = new AtomicInteger(0); - client.setVerifier((action, request, listener) -> verifyComponentTemplateInstalled(calledTimes, action, request, listener)); - registry.clusterChanged(event); - assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getComponentTemplateConfigs().size()))); - - calledTimes.set(0); - - // attempting to register the event multiple times as a race condition can yield this test flaky, namely: - // when calling registry.clusterChanged(newEvent) the templateCreationsInProgress state that the IndexTemplateRegistry maintains - // might've not yet been updated to reflect that the first template registration was complete, so a second template registration - // will not be issued anymore, leaving calledTimes to 0 - assertBusy(() -> { - // now delete all templates from the cluster state and let's retry - ClusterChangedEvent newEvent = createClusterChangedEvent(Collections.emptyMap(), Collections.emptyMap(), nodes); - registry.clusterChanged(newEvent); - assertThat(calledTimes.get(), greaterThan(4)); - }); - } - - public void testThatVersionedOldComponentTemplatesAreUpgraded() throws Exception { - DiscoveryNode node = DiscoveryNodeUtils.create("node"); - DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); - - ClusterChangedEvent event = createClusterChangedEvent( - Collections.emptyMap(), - Collections.singletonMap( - ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-settings", - ConnectorTemplateRegistry.REGISTRY_VERSION - 1 - ), - Collections.singletonMap(ConnectorTemplateRegistry.SEARCH_DEFAULT_PIPELINE_NAME, ConnectorTemplateRegistry.REGISTRY_VERSION), - Collections.emptyMap(), - nodes - ); - AtomicInteger calledTimes = new AtomicInteger(0); - client.setVerifier((action, request, listener) -> verifyComponentTemplateInstalled(calledTimes, action, request, listener)); - registry.clusterChanged(event); - assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getComponentTemplateConfigs().size()))); - } - - public void testThatUnversionedOldComponentTemplatesAreUpgraded() throws Exception { - DiscoveryNode node = DiscoveryNodeUtils.create("node"); - DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); - - ClusterChangedEvent event = createClusterChangedEvent( - Collections.emptyMap(), - Collections.singletonMap(ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-mappings", null), - Collections.singletonMap(ConnectorTemplateRegistry.SEARCH_DEFAULT_PIPELINE_NAME, ConnectorTemplateRegistry.REGISTRY_VERSION), - Collections.emptyMap(), - nodes - ); - AtomicInteger calledTimes = new AtomicInteger(0); - client.setVerifier((action, request, listener) -> verifyComponentTemplateInstalled(calledTimes, action, request, listener)); - registry.clusterChanged(event); - assertBusy(() -> assertThat(calledTimes.get(), equalTo(registry.getComponentTemplateConfigs().size()))); - } - - public void testSameOrHigherVersionComponentTemplateNotUpgraded() { - DiscoveryNode node = DiscoveryNodeUtils.create("node"); - DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").masterNodeId("node").add(node).build(); - - Map versions = new HashMap<>(); - versions.put(ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-mappings", ConnectorTemplateRegistry.REGISTRY_VERSION); - versions.put(ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-settings", ConnectorTemplateRegistry.REGISTRY_VERSION); - versions.put(ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-mappings", ConnectorTemplateRegistry.REGISTRY_VERSION); - versions.put(ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-settings", ConnectorTemplateRegistry.REGISTRY_VERSION); - versions.put(ConnectorTemplateRegistry.ACCESS_CONTROL_TEMPLATE_NAME, ConnectorTemplateRegistry.REGISTRY_VERSION); - ClusterChangedEvent sameVersionEvent = createClusterChangedEvent(Collections.emptyMap(), versions, nodes); - client.setVerifier((action, request, listener) -> { - if (action == PutPipelineTransportAction.TYPE) { - // Ignore this, it's verified in another test - return AcknowledgedResponse.TRUE; - } - if (action instanceof PutComponentTemplateAction) { - fail("template should not have been re-installed"); - return null; - } else if (action == ILMActions.PUT) { - // Ignore this, it's verified in another test - return AcknowledgedResponse.TRUE; - } else if (action == TransportPutComposableIndexTemplateAction.TYPE) { - // Ignore this, it's verified in another test - return AcknowledgedResponse.TRUE; - } else { - fail("client called with unexpected request:" + request.toString()); - return null; - } - }); - registry.clusterChanged(sameVersionEvent); - - versions.clear(); - versions.put( - ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-mappings", - ConnectorTemplateRegistry.REGISTRY_VERSION + randomIntBetween(0, 1000) - ); - versions.put( - ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME + "-settings", - ConnectorTemplateRegistry.REGISTRY_VERSION + randomIntBetween(0, 1000) - ); - versions.put( - ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-mappings", - ConnectorTemplateRegistry.REGISTRY_VERSION + randomIntBetween(0, 1000) - ); - versions.put( - ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME + "-settings", - ConnectorTemplateRegistry.REGISTRY_VERSION + randomIntBetween(0, 1000) - ); - versions.put( - ConnectorTemplateRegistry.ACCESS_CONTROL_TEMPLATE_NAME, - ConnectorTemplateRegistry.REGISTRY_VERSION + randomIntBetween(0, 1000) - ); - ClusterChangedEvent higherVersionEvent = createClusterChangedEvent(Collections.emptyMap(), versions, nodes); - registry.clusterChanged(higherVersionEvent); - } - public void testThatMissingMasterNodeDoesNothing() { DiscoveryNode localNode = DiscoveryNodeUtils.create("node"); DiscoveryNodes nodes = DiscoveryNodes.builder().localNodeId("node").add(localNode).build(); @@ -260,7 +125,7 @@ public void testThatMissingMasterNodeDoesNothing() { }); ClusterChangedEvent event = createClusterChangedEvent( - Collections.singletonMap(ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME, null), + Collections.singletonMap(ConnectorTemplateRegistry.ACCESS_CONTROL_TEMPLATE_NAME, null), Collections.emptyMap(), nodes ); @@ -357,10 +222,7 @@ private ActionResponse verifyComposableTemplateInstalled( assertThat(putRequest.indexTemplate().version(), equalTo((long) ConnectorTemplateRegistry.REGISTRY_VERSION)); final List indexPatterns = putRequest.indexTemplate().indexPatterns(); assertThat(indexPatterns, hasSize(1)); - assertThat( - indexPatterns, - contains(oneOf(ACCESS_CONTROL_INDEX_NAME_PATTERN, CONNECTOR_INDEX_NAME_PATTERN, CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN)) - ); + assertThat(indexPatterns, contains(ACCESS_CONTROL_INDEX_NAME_PATTERN)); assertNotNull(listener); return new TestPutIndexTemplateResponse(true); } else { diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTestUtils.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTestUtils.java index c563bc0a14ee3..3f2f47e190882 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTestUtils.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/ConnectorTestUtils.java @@ -7,7 +7,6 @@ package org.elasticsearch.xpack.application.connector; -import org.elasticsearch.client.internal.IndicesAdminClient; import org.elasticsearch.common.bytes.BytesReference; import org.elasticsearch.common.xcontent.XContentHelper; import org.elasticsearch.xcontent.XContentType; @@ -27,7 +26,6 @@ import org.elasticsearch.xpack.application.connector.filtering.FilteringValidation; import org.elasticsearch.xpack.application.connector.filtering.FilteringValidationInfo; import org.elasticsearch.xpack.application.connector.filtering.FilteringValidationState; -import org.elasticsearch.xpack.application.connector.syncjob.ConnectorSyncJob; import org.elasticsearch.xpack.application.connector.syncjob.ConnectorSyncJobType; import org.elasticsearch.xpack.core.scheduler.Cron; @@ -47,55 +45,14 @@ import static org.elasticsearch.test.ESTestCase.randomFrom; import static org.elasticsearch.test.ESTestCase.randomInt; import static org.elasticsearch.test.ESTestCase.randomList; -import static org.elasticsearch.test.ESTestCase.randomLong; import static org.elasticsearch.test.ESTestCase.randomLongBetween; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_INDEX_NAME_PATTERN; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_SYNC_JOBS_TEMPLATE_NAME; -import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.CONNECTOR_TEMPLATE_NAME; +import static org.elasticsearch.test.ESTestCase.randomNonNegativeLong; +import static org.elasticsearch.test.ESTestCase.randomShort; public final class ConnectorTestUtils { public static final String NULL_STRING = null; - /** - * Registers index templates for instances of {@link Connector} and {@link ConnectorSyncJob} with essential field mappings. This method - * only includes mappings for fields relevant to test cases, specifying field types to ensure correct ES query logic behavior. - * - * @param indicesAdminClient The Elasticsearch indices admin client used for template registration. - */ - - public static void registerSimplifiedConnectorIndexTemplates(IndicesAdminClient indicesAdminClient) { - - indicesAdminClient.preparePutTemplate(CONNECTOR_TEMPLATE_NAME) - .setPatterns(List.of(CONNECTOR_INDEX_NAME_PATTERN)) - .setVersion(0) - .setMapping( - "service_type", - "type=keyword,store=true", - "status", - "type=keyword,store=true", - "index_name", - "type=keyword,store=true", - "configuration", - "type=object" - ) - .get(); - - indicesAdminClient.preparePutTemplate(CONNECTOR_SYNC_JOBS_TEMPLATE_NAME) - .setPatterns(List.of(CONNECTOR_SYNC_JOBS_INDEX_NAME_PATTERN)) - .setVersion(0) - .setMapping( - "job_type", - "type=keyword,store=true", - "connector.id", - "type=keyword,store=true", - "status", - "type=keyword,store=true" - ) - .get(); - } - public static PutConnectorAction.Request getRandomPutConnectorActionRequest() { return new PutConnectorAction.Request( randomAlphaOfLengthBetween(5, 15), @@ -144,9 +101,9 @@ public static ConnectorSyncInfo getRandomConnectorSyncInfo() { return new ConnectorSyncInfo.Builder().setLastAccessControlSyncError(randomFrom(new String[] { null, randomAlphaOfLength(10) })) .setLastAccessControlSyncScheduledAt(randomFrom(new Instant[] { null, ConnectorTestUtils.randomInstant() })) .setLastAccessControlSyncStatus(randomFrom(new ConnectorSyncStatus[] { null, getRandomSyncStatus() })) - .setLastDeletedDocumentCount(randomLong()) + .setLastDeletedDocumentCount(randomNonNegativeLong()) .setLastIncrementalSyncScheduledAt(randomFrom(new Instant[] { null, ConnectorTestUtils.randomInstant() })) - .setLastIndexedDocumentCount(randomLong()) + .setLastIndexedDocumentCount(randomNonNegativeLong()) .setLastSyncError(randomFrom(new String[] { null, randomAlphaOfLength(10) })) .setLastSyncScheduledAt(randomFrom(new Instant[] { null, ConnectorTestUtils.randomInstant() })) .setLastSyncStatus(randomFrom(new ConnectorSyncStatus[] { null, getRandomSyncStatus() })) @@ -197,7 +154,7 @@ private static FilteringValidation getRandomFilteringValidationError() { public static ConnectorFiltering getRandomConnectorFiltering() { Instant currentTimestamp = Instant.now(); - int order = randomInt(); + int order = randomShort(); return new ConnectorFiltering.Builder().setActive( new FilteringRules.Builder().setAdvancedSnippet( diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexServiceTests.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexServiceTests.java index f6c0a54f107b4..fe6d97a871e0c 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexServiceTests.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobIndexServiceTests.java @@ -20,8 +20,11 @@ import org.elasticsearch.action.support.WriteRequest; import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.action.update.UpdateResponse; +import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.reindex.BulkByScrollResponse; +import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.plugins.Plugin; +import org.elasticsearch.plugins.SystemIndexPlugin; import org.elasticsearch.reindex.ReindexPlugin; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.test.ESSingleNodeTestCase; @@ -58,7 +61,6 @@ import static org.elasticsearch.xcontent.XContentFactory.jsonBuilder; import static org.elasticsearch.xpack.application.connector.ConnectorTemplateRegistry.ACCESS_CONTROL_INDEX_PREFIX; -import static org.elasticsearch.xpack.application.connector.ConnectorTestUtils.registerSimplifiedConnectorIndexTemplates; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThanOrEqualTo; @@ -86,14 +88,12 @@ protected Collection> getPlugins() { List> plugins = new ArrayList<>(super.getPlugins()); // Reindex plugin is required for testDeleteAllSyncJobsByConnectorId (supports delete_by_query) plugins.add(ReindexPlugin.class); + plugins.add(TestPlugin.class); return plugins; } @Before public void setup() throws Exception { - - registerSimplifiedConnectorIndexTemplates(indicesAdmin()); - connectorOneId = createConnector(ConnectorTestUtils.getRandomConnector()); connectorTwoId = createConnector(ConnectorTestUtils.getRandomConnector()); connectorThreeId = createConnector(ConnectorTestUtils.getRandomConnectorWithDetachedIndex()); @@ -805,18 +805,18 @@ public void testUpdateConnectorSyncJobIngestionStats() throws Exception { Instant requestLastSeen = request.getLastSeen(); Map metadata = request.getMetadata(); - Long deletedDocumentCountAfterUpdate = (Long) syncJobSourceAfterUpdate.get( + Long deletedDocumentCountAfterUpdate = ((Number) syncJobSourceAfterUpdate.get( ConnectorSyncJob.DELETED_DOCUMENT_COUNT_FIELD.getPreferredName() - ); - Long indexedDocumentCountAfterUpdate = (Long) syncJobSourceAfterUpdate.get( + )).longValue(); + Long indexedDocumentCountAfterUpdate = ((Number) syncJobSourceAfterUpdate.get( ConnectorSyncJob.INDEXED_DOCUMENT_COUNT_FIELD.getPreferredName() - ); - Long indexedDocumentVolumeAfterUpdate = (Long) syncJobSourceAfterUpdate.get( + )).longValue(); + Long indexedDocumentVolumeAfterUpdate = ((Number) syncJobSourceAfterUpdate.get( ConnectorSyncJob.INDEXED_DOCUMENT_VOLUME_FIELD.getPreferredName() - ); - Long totalDocumentCountAfterUpdate = (Long) syncJobSourceAfterUpdate.get( + )).longValue(); + Long totalDocumentCountAfterUpdate = ((Number) syncJobSourceAfterUpdate.get( ConnectorSyncJob.TOTAL_DOCUMENT_COUNT_FIELD.getPreferredName() - ); + )).longValue(); Instant lastSeenAfterUpdate = Instant.parse( (String) syncJobSourceAfterUpdate.get(ConnectorSyncJob.LAST_SEEN_FIELD.getPreferredName()) ); @@ -1411,4 +1411,24 @@ private String updateConnectorSyncJobStatusWithoutStateMachineGuard(String syncJ // wait 10 seconds for connector creation return index.get(TIMEOUT_SECONDS, TimeUnit.SECONDS).getId(); } + + /** + * Test plugin to register the {@link ConnectorSyncJobIndexService} system index descriptor. + */ + public static class TestPlugin extends Plugin implements SystemIndexPlugin { + @Override + public Collection getSystemIndexDescriptors(Settings settings) { + return List.of(ConnectorSyncJobIndexService.getSystemIndexDescriptor()); + } + + @Override + public String getFeatureName() { + return this.getClass().getSimpleName(); + } + + @Override + public String getFeatureDescription() { + return this.getClass().getCanonicalName(); + } + } } diff --git a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobTestUtils.java b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobTestUtils.java index e72bf04fb7e55..1e6426051e04b 100644 --- a/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobTestUtils.java +++ b/x-pack/plugin/ent-search/src/test/java/org/elasticsearch/xpack/application/connector/syncjob/ConnectorSyncJobTestUtils.java @@ -36,7 +36,7 @@ import static org.elasticsearch.test.ESTestCase.randomInt; import static org.elasticsearch.test.ESTestCase.randomLong; import static org.elasticsearch.test.ESTestCase.randomMap; -import static org.elasticsearch.test.ESTestCase.randomNonNegativeLong; +import static org.elasticsearch.test.ESTestCase.randomNonNegativeInt; public class ConnectorSyncJobTestUtils { @@ -51,11 +51,11 @@ public static ConnectorSyncJob getRandomConnectorSyncJob() { .setCompletedAt(randomFrom(new Instant[] { null, randomInstantBetween(lowerBoundInstant, upperBoundInstant) })) .setConnector(ConnectorTestUtils.getRandomSyncJobConnectorInfo()) .setCreatedAt(randomInstantBetween(lowerBoundInstant, upperBoundInstant)) - .setDeletedDocumentCount(randomLong()) + .setDeletedDocumentCount(randomNonNegativeInt()) .setError(randomFrom(new String[] { null, randomAlphaOfLength(10) })) .setId(randomAlphaOfLength(10)) - .setIndexedDocumentCount(randomLong()) - .setIndexedDocumentVolume(randomLong()) + .setIndexedDocumentCount(randomNonNegativeInt()) + .setIndexedDocumentVolume(randomNonNegativeInt()) .setJobType(getRandomConnectorJobType()) .setLastSeen(randomFrom(new Instant[] { null, randomInstantBetween(lowerBoundInstant, upperBoundInstant) })) .setMetadata( @@ -67,7 +67,7 @@ public static ConnectorSyncJob getRandomConnectorSyncJob() { ) .setStartedAt(randomFrom(new Instant[] { null, randomInstantBetween(lowerBoundInstant, upperBoundInstant) })) .setStatus(ConnectorTestUtils.getRandomSyncStatus()) - .setTotalDocumentCount(randomLong()) + .setTotalDocumentCount(randomNonNegativeInt()) .setTriggerMethod(getRandomConnectorSyncJobTriggerMethod()) .setWorkerHostname(randomAlphaOfLength(10)) .build(); @@ -156,10 +156,10 @@ public static UpdateConnectorSyncJobIngestionStatsAction.Request getRandomUpdate return new UpdateConnectorSyncJobIngestionStatsAction.Request( randomAlphaOfLength(10), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), randomInstantBetween(lowerBoundInstant, upperBoundInstant), randomMap(2, 3, () -> new Tuple<>(randomAlphaOfLength(4), randomAlphaOfLength(4))) ); @@ -173,10 +173,10 @@ public static UpdateConnectorSyncJobIngestionStatsAction.Request getRandomUpdate return new UpdateConnectorSyncJobIngestionStatsAction.Request( syncJobId, - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), - randomNonNegativeLong(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), + (long) randomNonNegativeInt(), randomInstantBetween(lowerBoundInstant, upperBoundInstant), randomMap(2, 3, () -> new Tuple<>(randomAlphaOfLength(4), randomAlphaOfLength(4))) ); From 39c4eda6ce362ba75363f83523d61e242c45a2dc Mon Sep 17 00:00:00 2001 From: Quentin Pradet Date: Fri, 24 Jan 2025 16:31:30 +0400 Subject: [PATCH 03/23] Add back keep_alive to async_search.submit rest-api-spec (#120781) --- docs/changelog/120781.yaml | 5 +++++ .../resources/rest-api-spec/api/async_search.submit.json | 5 +++++ 2 files changed, 10 insertions(+) create mode 100644 docs/changelog/120781.yaml diff --git a/docs/changelog/120781.yaml b/docs/changelog/120781.yaml new file mode 100644 index 0000000000000..67c7d90528d6e --- /dev/null +++ b/docs/changelog/120781.yaml @@ -0,0 +1,5 @@ +pr: 120781 +summary: Add back `keep_alive` to `async_search.submit` rest-api-spec +area: Search +type: bug +issues: [] diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/async_search.submit.json b/rest-api-spec/src/main/resources/rest-api-spec/api/async_search.submit.json index 3de0dec85f547..8ae2fff22281c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/async_search.submit.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/async_search.submit.json @@ -43,6 +43,11 @@ "description":"Control whether the response should be stored in the cluster if it completed within the provided [wait_for_completion] time (default: false)", "default":false }, + "keep_alive": { + "type": "time", + "description": "Update the time interval in which the results (partial or final) for this search will be available", + "default": "5d" + }, "batched_reduce_size":{ "type":"number", "description":"The number of shard results that should be reduced at once on the coordinating node. This value should be used as the granularity at which progress results will be made available.", From 32eada688f7de63c75a22c4b9f6464aab75c01b7 Mon Sep 17 00:00:00 2001 From: Alexey Ivanov Date: Fri, 24 Jan 2025 12:41:00 +0000 Subject: [PATCH 04/23] Introduce minimumCompatibilityVersion to BuildVersion (ES-9378) (#119101) --- .../org/elasticsearch/ReleaseVersions.java | 5 +-- .../main/java/org/elasticsearch/Version.java | 20 ++++----- .../elasticsearch/cluster/ClusterState.java | 7 ---- .../org/elasticsearch/env/BuildVersion.java | 6 +++ .../env/DefaultBuildVersion.java | 5 +++ .../elasticsearch/env/NodeEnvironment.java | 29 ++++--------- .../org/elasticsearch/env/NodeMetadata.java | 4 +- .../elasticsearch/env/BuildVersionTests.java | 10 +++++ .../env/NodeEnvironmentTests.java | 41 ++++++++++++------- .../elasticsearch/env/NodeMetadataTests.java | 28 ++++++------- .../index/IndexVersionTests.java | 1 + .../index/mapper/MapperTestCase.java | 2 - 12 files changed, 81 insertions(+), 77 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ReleaseVersions.java b/server/src/main/java/org/elasticsearch/ReleaseVersions.java index 09d6bdd1b4799..22cd18c7b4ac3 100644 --- a/server/src/main/java/org/elasticsearch/ReleaseVersions.java +++ b/server/src/main/java/org/elasticsearch/ReleaseVersions.java @@ -10,7 +10,7 @@ package org.elasticsearch; import org.elasticsearch.common.Strings; -import org.elasticsearch.core.UpdateForV9; +import org.elasticsearch.core.UpdateForV10; import org.elasticsearch.internal.BuildExtension; import org.elasticsearch.plugins.ExtensionLoader; @@ -114,8 +114,7 @@ private static IntFunction lookupFunction(NavigableMap, ToXContentFragment { VERSION_STRINGS = Map.copyOf(builderByString); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) - // Re-enable this assertion once the rest api version is bumped private static void assertRestApiVersion() { - // assert RestApiVersion.current().major == CURRENT.major && RestApiVersion.previous().major == CURRENT.major - 1 - // : "RestApiVersion must be upgraded " - // + "to reflect major from Version.CURRENT [" - // + CURRENT.major - // + "]" - // + " but is still set to [" - // + RestApiVersion.current().major - // + "]"; + assert RestApiVersion.current().major == CURRENT.major && RestApiVersion.previous().major == CURRENT.major - 1 + : "RestApiVersion must be upgraded " + + "to reflect major from Version.CURRENT [" + + CURRENT.major + + "]" + + " but is still set to [" + + RestApiVersion.current().major + + "]"; } public static Version readVersion(StreamInput in) throws IOException { diff --git a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java index 62f2947d06a41..6b222fb8f5bdc 100644 --- a/server/src/main/java/org/elasticsearch/cluster/ClusterState.java +++ b/server/src/main/java/org/elasticsearch/cluster/ClusterState.java @@ -47,7 +47,6 @@ import org.elasticsearch.common.xcontent.ChunkedToXContentHelper; import org.elasticsearch.core.Nullable; import org.elasticsearch.core.SuppressForbidden; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.index.shard.IndexLongFieldRange; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.xcontent.ToXContent; @@ -1039,12 +1038,6 @@ public static ClusterState readFrom(StreamInput in, DiscoveryNode localNode) thr return builder.build(); } - /** - * If the cluster state does not contain transport version information, this is the version - * that is inferred for all nodes on version 8.8.0 or above. - */ - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) - public static final TransportVersion INFERRED_TRANSPORT_VERSION = TransportVersions.V_8_8_0; public static final Version VERSION_INTRODUCING_TRANSPORT_VERSIONS = Version.V_8_8_0; @Override diff --git a/server/src/main/java/org/elasticsearch/env/BuildVersion.java b/server/src/main/java/org/elasticsearch/env/BuildVersion.java index 5c3602283fef3..877a07d9a3ee5 100644 --- a/server/src/main/java/org/elasticsearch/env/BuildVersion.java +++ b/server/src/main/java/org/elasticsearch/env/BuildVersion.java @@ -73,6 +73,12 @@ public abstract class BuildVersion implements ToXContentFragment, Writeable { */ public abstract String toNodeMetadata(); + /** + * Returns the minimum compatible build version based on the current version. + * Ie a node needs to have at least the return version in order to communicate with a node running the current version. + */ + public abstract BuildVersion minimumCompatibilityVersion(); + /** * Create a {@link BuildVersion} from a version ID number. * diff --git a/server/src/main/java/org/elasticsearch/env/DefaultBuildVersion.java b/server/src/main/java/org/elasticsearch/env/DefaultBuildVersion.java index 913a352debfb8..39b9278a78c97 100644 --- a/server/src/main/java/org/elasticsearch/env/DefaultBuildVersion.java +++ b/server/src/main/java/org/elasticsearch/env/DefaultBuildVersion.java @@ -73,6 +73,11 @@ public String toNodeMetadata() { return Integer.toString(version.id()); } + @Override + public BuildVersion minimumCompatibilityVersion() { + return fromVersionId(version.minimumCompatibilityVersion().id); + } + @Override public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { return builder.value(version.id()); diff --git a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java index afadb8f5b3011..90e2ae5c62703 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java +++ b/server/src/main/java/org/elasticsearch/env/NodeEnvironment.java @@ -86,8 +86,6 @@ import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; -import java.util.regex.Matcher; -import java.util.regex.Pattern; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -525,7 +523,7 @@ static void checkForIndexCompatibility(Logger logger, DataPath... dataPaths) thr logger.info("oldest index version recorded in NodeMetadata {}", metadata.oldestIndexVersion()); if (metadata.oldestIndexVersion().before(IndexVersions.MINIMUM_COMPATIBLE)) { - String bestDowngradeVersion = getBestDowngradeVersion(metadata.previousNodeVersion().toString()); + BuildVersion bestDowngradeVersion = getBestDowngradeVersion(metadata.previousNodeVersion()); throw new IllegalStateException( "Cannot start this node because it holds metadata for indices with version [" + metadata.oldestIndexVersion().toReleaseVersion() @@ -1504,28 +1502,17 @@ private static void tryWriteTempFile(Path path) throws IOException { /** * Get a useful version string to direct a user's downgrade operation * - *

If a user is trying to install 8.0 but has incompatible indices, the user should - * downgrade to 7.17.x. We return 7.17.0, unless the user is trying to upgrade from - * a 7.17.x release, in which case we return the last installed version. + *

If a user is trying to install current major N but has incompatible indices, the user should + * downgrade to last minor of the previous major (N-1).last. We return (N-1).last, unless the user is trying to upgrade from + * a (N-1).last.x release, in which case we return the last installed version. * @return Version to downgrade to */ // visible for testing - static String getBestDowngradeVersion(String previousNodeVersion) { - // this method should only be called in the context of an upgrade to 8.x - assert Build.current().version().startsWith("9.") == false; - Pattern pattern = Pattern.compile("^7\\.(\\d+)\\.\\d+$"); - Matcher matcher = pattern.matcher(previousNodeVersion); - if (matcher.matches()) { - try { - int minorVersion = Integer.parseInt(matcher.group(1)); - if (minorVersion >= 17) { - return previousNodeVersion; - } - } catch (NumberFormatException e) { - // continue and return default - } + static BuildVersion getBestDowngradeVersion(BuildVersion previousNodeVersion) { + if (previousNodeVersion.onOrAfterMinimumCompatible()) { + return previousNodeVersion; } - return "7.17.0"; + return BuildVersion.current().minimumCompatibilityVersion(); } } diff --git a/server/src/main/java/org/elasticsearch/env/NodeMetadata.java b/server/src/main/java/org/elasticsearch/env/NodeMetadata.java index c71a3798be1f7..48268b5001f3e 100644 --- a/server/src/main/java/org/elasticsearch/env/NodeMetadata.java +++ b/server/src/main/java/org/elasticsearch/env/NodeMetadata.java @@ -10,7 +10,6 @@ package org.elasticsearch.env; import org.elasticsearch.Build; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.gateway.MetadataStateFormat; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.IndexVersions; @@ -158,12 +157,11 @@ public void setOldestIndexVersion(int oldestIndexVersion) { this.oldestIndexVersion = IndexVersion.fromId(oldestIndexVersion); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // version is required in the node metadata from v9 onwards public NodeMetadata build() { final IndexVersion oldestIndexVersion; if (this.nodeVersion == null) { - nodeVersion = BuildVersion.fromVersionId(0); + throw new IllegalStateException("Node version is required in node metadata"); } if (this.previousNodeVersion == null) { previousNodeVersion = nodeVersion; diff --git a/server/src/test/java/org/elasticsearch/env/BuildVersionTests.java b/server/src/test/java/org/elasticsearch/env/BuildVersionTests.java index 9fd889426fd2d..96875ac1a95e5 100644 --- a/server/src/test/java/org/elasticsearch/env/BuildVersionTests.java +++ b/server/src/test/java/org/elasticsearch/env/BuildVersionTests.java @@ -43,6 +43,16 @@ public void testIsFutureVersion() { assertTrue(futureVersion.isFutureVersion()); } + public void testMinimumCompatibilityVersion() { + BuildVersion minCompatible = BuildVersion.fromVersionId(Version.CURRENT.minimumCompatibilityVersion().id()); + assertThat(BuildVersion.current().minimumCompatibilityVersion(), equalTo(minCompatible)); + + BuildVersion previousCompatible = BuildVersion.fromVersionId( + Version.CURRENT.minimumCompatibilityVersion().minimumCompatibilityVersion().id() + ); + assertThat(minCompatible.minimumCompatibilityVersion(), equalTo(previousCompatible)); + } + public static BuildVersion increment(BuildVersion version) { return BuildVersion.fromVersionId(((DefaultBuildVersion) version).version.id() + 1); } diff --git a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java index 42a94ebf8c6ff..82934bda2d259 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeEnvironmentTests.java @@ -584,7 +584,9 @@ public void testIndexCompatibilityChecks() throws IOException { containsString("it holds metadata for indices with version [" + oldIndexVersion.toReleaseVersion() + "]"), containsString( "Revert this node to version [" - + (previousNodeVersion.major == Version.V_8_0_0.major ? Version.V_7_17_0 : previousNodeVersion) + + (previousNodeVersion.major == Version.CURRENT.major + ? Version.CURRENT.minimumCompatibilityVersion() + : previousNodeVersion) + "]" ) ) @@ -638,20 +640,31 @@ public void testSymlinkDataDirectory() throws Exception { env.close(); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) - @AwaitsFix(bugUrl = "test won't work until we remove and bump minimum index versions") public void testGetBestDowngradeVersion() { - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.0"), Matchers.equalTo("7.17.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.5"), Matchers.equalTo("7.17.5")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.1234"), Matchers.equalTo("7.17.1234")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.18.0"), Matchers.equalTo("7.18.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.x"), Matchers.equalTo("7.17.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.5-SNAPSHOT"), Matchers.equalTo("7.17.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.17.6b"), Matchers.equalTo("7.17.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("7.16.0"), Matchers.equalTo("7.17.0")); - // when we get to version 7.2147483648.0 we will have to rethink our approach, but for now we return 7.17.0 with an integer overflow - assertThat(NodeEnvironment.getBestDowngradeVersion("7." + Integer.MAX_VALUE + "0.0"), Matchers.equalTo("7.17.0")); - assertThat(NodeEnvironment.getBestDowngradeVersion("foo"), Matchers.equalTo("7.17.0")); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.0")), + Matchers.equalTo(BuildVersion.fromString("8.18.0")) + ); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.5")), + Matchers.equalTo(BuildVersion.fromString("8.18.5")) + ); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.18.12")), + Matchers.equalTo(BuildVersion.fromString("8.18.12")) + ); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.19.0")), + Matchers.equalTo(BuildVersion.fromString("8.19.0")) + ); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("8.17.0")), + Matchers.equalTo(BuildVersion.fromString("8.18.0")) + ); + assertThat( + NodeEnvironment.getBestDowngradeVersion(BuildVersion.fromString("7.17.0")), + Matchers.equalTo(BuildVersion.fromString("8.18.0")) + ); } private void verifyFailsOnShardData(Settings settings, Path indexPath, String shardDataDirName) { diff --git a/server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java b/server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java index eccdd1c6ffea7..dab2932369c21 100644 --- a/server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java +++ b/server/src/test/java/org/elasticsearch/env/NodeMetadataTests.java @@ -9,12 +9,11 @@ package org.elasticsearch.env; import org.elasticsearch.Build; +import org.elasticsearch.ElasticsearchException; import org.elasticsearch.Version; import org.elasticsearch.core.Tuple; -import org.elasticsearch.core.UpdateForV9; import org.elasticsearch.gateway.MetadataStateFormat; import org.elasticsearch.index.IndexVersion; -import org.elasticsearch.index.IndexVersions; import org.elasticsearch.test.ESTestCase; import org.elasticsearch.test.EqualsHashCodeTestUtils; import org.elasticsearch.test.VersionUtils; @@ -28,6 +27,7 @@ import static org.hamcrest.Matchers.allOf; import static org.hamcrest.Matchers.endsWith; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.startsWith; @@ -80,22 +80,20 @@ public void testEqualsHashcodeSerialization() { ); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) - @AwaitsFix(bugUrl = "as mentioned in the comment below, the behavior here is changing for 9.0 so this test needs updating") - public void testReadsFormatWithoutVersion() throws IOException { - // the behaviour tested here is only appropriate if the current version is compatible with versions 7 and earlier - assertTrue(IndexVersions.MINIMUM_COMPATIBLE.onOrBefore(IndexVersions.V_7_0_0)); - // when the current version is incompatible with version 7, the behaviour should change to reject files like the given resource - // which do not have the version field - + public void testFailsToReadFormatWithoutVersion() throws IOException { final Path tempDir = createTempDir(); final Path stateDir = Files.createDirectory(tempDir.resolve(MetadataStateFormat.STATE_DIR_NAME)); final InputStream resource = this.getClass().getResourceAsStream("testReadsFormatWithoutVersion.binary"); assertThat(resource, notNullValue()); Files.copy(resource, stateDir.resolve(NodeMetadata.FORMAT.getStateFileName(between(0, Integer.MAX_VALUE)))); - final NodeMetadata nodeMetadata = NodeMetadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempDir); - assertThat(nodeMetadata.nodeId(), equalTo("y6VUVMSaStO4Tz-B5BxcOw")); - assertThat(nodeMetadata.nodeVersion(), equalTo(BuildVersion.fromVersionId(0))); + + ElasticsearchException ex = expectThrows( + ElasticsearchException.class, + () -> NodeMetadata.FORMAT.loadLatestState(logger, xContentRegistry(), tempDir) + ); + Throwable rootCause = ex.getRootCause(); + assertThat(rootCause, instanceOf(IllegalStateException.class)); + assertThat("Node version is required in node metadata", equalTo(rootCause.getMessage())); } public void testUpgradesLegitimateVersions() { @@ -155,11 +153,9 @@ public void testDoesNotUpgradeAncientVersion() { ); } - @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) - @AwaitsFix(bugUrl = "Needs to be updated for 9.0 version bump") public void testUpgradeMarksPreviousVersion() { final String nodeId = randomAlphaOfLength(10); - final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_8_0_0); + final Version version = VersionUtils.randomVersionBetween(random(), Version.CURRENT.minimumCompatibilityVersion(), Version.V_9_0_0); final BuildVersion buildVersion = BuildVersion.fromVersionId(version.id()); final NodeMetadata nodeMetadata = new NodeMetadata(nodeId, buildVersion, IndexVersion.current()).upgradeToCurrentVersion(); diff --git a/server/src/test/java/org/elasticsearch/index/IndexVersionTests.java b/server/src/test/java/org/elasticsearch/index/IndexVersionTests.java index 8575b87c36799..f6ebd33aae9dd 100644 --- a/server/src/test/java/org/elasticsearch/index/IndexVersionTests.java +++ b/server/src/test/java/org/elasticsearch/index/IndexVersionTests.java @@ -153,6 +153,7 @@ public void testMax() { public void testGetMinimumCompatibleIndexVersion() { assertThat(IndexVersion.getMinimumCompatibleIndexVersion(7170099), equalTo(IndexVersion.fromId(6000099))); assertThat(IndexVersion.getMinimumCompatibleIndexVersion(8000099), equalTo(IndexVersion.fromId(7000099))); + assertThat(IndexVersion.getMinimumCompatibleIndexVersion(9000099), equalTo(IndexVersion.fromId(8000099))); assertThat(IndexVersion.getMinimumCompatibleIndexVersion(10000000), equalTo(IndexVersion.fromId(9000000))); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index a62af5729a096..809660c5e9af8 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -105,8 +105,6 @@ */ public abstract class MapperTestCase extends MapperServiceTestCase { - public static final IndexVersion DEPRECATED_BOOST_INDEX_VERSION = IndexVersions.V_7_10_0; - protected abstract void minimalMapping(XContentBuilder b) throws IOException; /** From 2fea5939cfa4ed5a945a08ea5cf334fa72c6cccb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tim=20R=C3=BChsen?= Date: Fri, 24 Jan 2025 14:45:35 +0100 Subject: [PATCH 05/23] [Profiling] Rename profiling.host.name to host.name in profiling-hosts (#120783) --- .../profiling/component-template/profiling-hosts.json | 6 +++--- .../resources/data/profiling-hosts.ndjson | 4 ++-- .../resources/rest-api-spec/test/profiling/10_basic.yml | 2 +- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/x-pack/plugin/core/template-resources/src/main/resources/profiling/component-template/profiling-hosts.json b/x-pack/plugin/core/template-resources/src/main/resources/profiling/component-template/profiling-hosts.json index b0c99bf8a0cea..e23ae42ce1f4d 100644 --- a/x-pack/plugin/core/template-resources/src/main/resources/profiling/component-template/profiling-hosts.json +++ b/x-pack/plugin/core/template-resources/src/main/resources/profiling/component-template/profiling-hosts.json @@ -40,6 +40,9 @@ "id": { "type": "keyword" }, + "name": { + "type": "keyword" + }, "type": { "type": "keyword" } @@ -177,9 +180,6 @@ "tags": { "type": "keyword" }, - "name": { - "type": "keyword" - }, "machine": { "type": "keyword" }, diff --git a/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-hosts.ndjson b/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-hosts.ndjson index e12a670a79d18..8cd5425c86994 100644 --- a/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-hosts.ndjson +++ b/x-pack/plugin/profiling/src/internalClusterTest/resources/data/profiling-hosts.ndjson @@ -1,4 +1,4 @@ {"create": {"_index": "profiling-hosts","_id":"eLH27YsBj2lLi3tJYlvr"}} -{"profiling.project.id":100,"host.id":"8457605156473051743","@timestamp":1700504426,"ecs.version":"1.12.0","profiling.agent.build_timestamp":1688111067,"profiling.instance.private_ipv4s":["192.168.1.2"],"ec2.instance_life_cycle":"on-demand","profiling.agent.config.map_scale_factor":0,"host.type":"i3.2xlarge","profiling.host.ip":"192.168.1.2","profiling.agent.config.bpf_log_level":0,"profiling.host.sysctl.net.core.bpf_jit_enable":1,"profiling.agent.config.file":"/etc/prodfiler/prodfiler.conf","ec2.local_ipv4":"192.168.1.2","profiling.agent.config.no_kernel_version_check":false,"host.arch":"amd64","profiling.host.tags":["cloud_provider:aws","cloud_environment:qa","cloud_region:eu-west-1"],"profiling.agent.config.probabilistic_threshold":100,"profiling.agent.config.disable_tls":false,"profiling.agent.config.tracers":"all","profiling.agent.start_time":1700090045589,"profiling.agent.config.max_elements_per_interval":800,"cloud.provider":"aws","cloud.region":"eu-west-1","profiling.agent.config.present_cpu_cores":8,"profiling.host.kernel_version":"9.9.9-0-aws","profiling.agent.config.bpf_log_size":65536,"profiling.agent.config.known_traces_entries":65536,"profiling.host.sysctl.kernel.unprivileged_bpf_disabled":1,"profiling.agent.config.verbose":false,"profiling.agent.config.probabilistic_interval":"1m0s","ec2.placement.availability_zone_id":"euw1-az1","ec2.security_groups":"","ec2.local_hostname":"ip-192-168-1-2.eu-west-1.compute.internal","ec2.placement.availability_zone":"eu-west-1c","profiling.agent.config.upload_symbols":false,"profiling.host.sysctl.kernel.bpf_stats_enabled":0,"profiling.host.name":"ip-192-168-1-2","ec2.mac":"00:11:22:33:44:55","profiling.host.kernel_proc_version":"Linux version 9.9.9-0-aws","profiling.agent.config.cache_directory":"/var/cache/optimyze/","profiling.agent.version":"v8.12.0","ec2.hostname":"ip-192-168-1-2.eu-west-1.compute.internal","profiling.agent.config.elastic_mode":false,"ec2.ami_id":"ami-aaaaaaaaaaa","ec2.instance_id":"i-0b999999999999999"} +{"profiling.project.id":100,"host.id":"8457605156473051743","@timestamp":1700504426,"ecs.version":"1.12.0","profiling.agent.build_timestamp":1688111067,"profiling.instance.private_ipv4s":["192.168.1.2"],"ec2.instance_life_cycle":"on-demand","profiling.agent.config.map_scale_factor":0,"host.type":"i3.2xlarge","profiling.host.ip":"192.168.1.2","profiling.agent.config.bpf_log_level":0,"profiling.host.sysctl.net.core.bpf_jit_enable":1,"profiling.agent.config.file":"/etc/prodfiler/prodfiler.conf","ec2.local_ipv4":"192.168.1.2","profiling.agent.config.no_kernel_version_check":false,"host.arch":"amd64","profiling.host.tags":["cloud_provider:aws","cloud_environment:qa","cloud_region:eu-west-1"],"profiling.agent.config.probabilistic_threshold":100,"profiling.agent.config.disable_tls":false,"profiling.agent.config.tracers":"all","profiling.agent.start_time":1700090045589,"profiling.agent.config.max_elements_per_interval":800,"cloud.provider":"aws","cloud.region":"eu-west-1","profiling.agent.config.present_cpu_cores":8,"profiling.host.kernel_version":"9.9.9-0-aws","profiling.agent.config.bpf_log_size":65536,"profiling.agent.config.known_traces_entries":65536,"profiling.host.sysctl.kernel.unprivileged_bpf_disabled":1,"profiling.agent.config.verbose":false,"profiling.agent.config.probabilistic_interval":"1m0s","ec2.placement.availability_zone_id":"euw1-az1","ec2.security_groups":"","ec2.local_hostname":"ip-192-168-1-2.eu-west-1.compute.internal","ec2.placement.availability_zone":"eu-west-1c","profiling.agent.config.upload_symbols":false,"profiling.host.sysctl.kernel.bpf_stats_enabled":0,"host.name":"ip-192-168-1-2","ec2.mac":"00:11:22:33:44:55","profiling.host.kernel_proc_version":"Linux version 9.9.9-0-aws","profiling.agent.config.cache_directory":"/var/cache/optimyze/","profiling.agent.version":"v8.12.0","ec2.hostname":"ip-192-168-1-2.eu-west-1.compute.internal","profiling.agent.config.elastic_mode":false,"ec2.ami_id":"ami-aaaaaaaaaaa","ec2.instance_id":"i-0b999999999999999"} {"create": {"_index": "profiling-hosts", "_id": "u_fHlYwBkmZvQ6tVo1Lr"}} -{"profiling.project.id":100,"host.id":"7416508186220657211","@timestamp":1703319912,"ecs.version":"1.12.0","profiling.agent.version":"8.11.0","profiling.agent.config.map_scale_factor":0,"profiling.agent.config.probabilistic_threshold":100,"profiling.host.name":"ip-192-186-1-3","profiling.agent.config.no_kernel_version_check":false,"profiling.host.sysctl.net.core.bpf_jit_enable":1,"profiling.agent.config.elastic_mode":false,"host.type":"Standard_D4s_v3","azure.compute.environment":"AzurePublicCloud","profiling.agent.config.bpf_log_level":0,"profiling.agent.config.known_traces_entries":65536,"profiling.agent.config.ca_address":"example.com:443","profiling.agent.config.tags":"cloud_provider:azure;cloud_environment:qa;cloud_region:eastus2","profiling.host.tags":["cloud_provider:azure","cloud_environment:qa","cloud_region:eastus2"],"profiling.host.kernel_version":"9.9.9-0-azure","profiling.agent.revision":"head-52cc2030","azure.compute.subscriptionid":"1-2-3-4-5","profiling.host.sysctl.kernel.bpf_stats_enabled":0,"host.arch":"amd64","azure.compute.zone":"3","profiling.agent.config.cache_directory":"/var/cache/Elastic/universal-profiling","azure.compute.name":"example-qa-eastus2-001-v1-zone3_6","profiling.agent.config.probabilistic_interval":"1m0s","cloud.provider":"azure","cloud.region":"eastus2","azure.compute.version":"1234.20230510.233254","profiling.instance.private_ipv4s":["192.168.1.3"],"profiling.agent.build_timestamp":1699000836,"profiling.agent.config.file":"/etc/Elastic/universal-profiling/pf-host-agent.conf","profiling.agent.config.bpf_log_size":65536,"profiling.host.sysctl.kernel.unprivileged_bpf_disabled":1,"profiling.agent.config.tracers":"all","profiling.agent.config.present_cpu_cores":4,"profiling.agent.start_time":1702306987358,"profiling.agent.config.disable_tls":false,"azure.compute.ostype":"Linux","profiling.host.ip":"192.168.1.3","profiling.agent.config.max_elements_per_interval":400,"profiling.agent.config.upload_symbols":false,"azure.compute.tags":"bootstrap-version:v1;ece-id:001;environment:qa;identifier:v1;initial-config:;managed-by:terraform;monitored-by:core-infrastructure;owner:core-infrastructure;region_type:ess;role:blueprint;secondary_role:;vars-identifier:eastus2-001-v1","profiling.host.kernel_proc_version":"Linux version 9.9.9-0-azure","profiling.agent.config.verbose":false,"azure.compute.vmid":"1-2-3-4-5"} +{"profiling.project.id":100,"host.id":"7416508186220657211","@timestamp":1703319912,"ecs.version":"1.12.0","profiling.agent.version":"8.11.0","profiling.agent.config.map_scale_factor":0,"profiling.agent.config.probabilistic_threshold":100,"host.name":"ip-192-186-1-3","profiling.agent.config.no_kernel_version_check":false,"profiling.host.sysctl.net.core.bpf_jit_enable":1,"profiling.agent.config.elastic_mode":false,"host.type":"Standard_D4s_v3","azure.compute.environment":"AzurePublicCloud","profiling.agent.config.bpf_log_level":0,"profiling.agent.config.known_traces_entries":65536,"profiling.agent.config.ca_address":"example.com:443","profiling.agent.config.tags":"cloud_provider:azure;cloud_environment:qa;cloud_region:eastus2","profiling.host.tags":["cloud_provider:azure","cloud_environment:qa","cloud_region:eastus2"],"profiling.host.kernel_version":"9.9.9-0-azure","profiling.agent.revision":"head-52cc2030","azure.compute.subscriptionid":"1-2-3-4-5","profiling.host.sysctl.kernel.bpf_stats_enabled":0,"host.arch":"amd64","azure.compute.zone":"3","profiling.agent.config.cache_directory":"/var/cache/Elastic/universal-profiling","azure.compute.name":"example-qa-eastus2-001-v1-zone3_6","profiling.agent.config.probabilistic_interval":"1m0s","cloud.provider":"azure","cloud.region":"eastus2","azure.compute.version":"1234.20230510.233254","profiling.instance.private_ipv4s":["192.168.1.3"],"profiling.agent.build_timestamp":1699000836,"profiling.agent.config.file":"/etc/Elastic/universal-profiling/pf-host-agent.conf","profiling.agent.config.bpf_log_size":65536,"profiling.host.sysctl.kernel.unprivileged_bpf_disabled":1,"profiling.agent.config.tracers":"all","profiling.agent.config.present_cpu_cores":4,"profiling.agent.start_time":1702306987358,"profiling.agent.config.disable_tls":false,"azure.compute.ostype":"Linux","profiling.host.ip":"192.168.1.3","profiling.agent.config.max_elements_per_interval":400,"profiling.agent.config.upload_symbols":false,"azure.compute.tags":"bootstrap-version:v1;ece-id:001;environment:qa;identifier:v1;initial-config:;managed-by:terraform;monitored-by:core-infrastructure;owner:core-infrastructure;region_type:ess;role:blueprint;secondary_role:;vars-identifier:eastus2-001-v1","profiling.host.kernel_proc_version":"Linux version 9.9.9-0-azure","profiling.agent.config.verbose":false,"azure.compute.vmid":"1-2-3-4-5"} diff --git a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/profiling/10_basic.yml b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/profiling/10_basic.yml index 4a9212b8a7158..7d93e7f6e08c7 100644 --- a/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/profiling/10_basic.yml +++ b/x-pack/plugin/src/yamlRestTest/resources/rest-api-spec/test/profiling/10_basic.yml @@ -116,7 +116,7 @@ setup: - {"create": {"_index": "profiling-executables", "_id": "lHp5_WAgpLy2alrUVab6HA"}} - {"@timestamp": "1698624000", "Executable": {"build": {"id": "c5f89ea1c68710d2a493bb604c343a92c4f8ddeb"}, "file": {"name": "vmlinux"}}, "Symbolization": {"next_time": "4852491791"}, "ecs": {"version": "1.12.0"}} - {"create": {"_index": "profiling-hosts", "_id": "eLH27YsBj2lLi3tJYlvr"}} - - {"profiling.project.id": 100, "host.id": "8457605156473051743", "@timestamp": 1700504426, "ecs.version": "1.12.0", "profiling.agent.build_timestamp": 1688111067, "profiling.instance.private_ipv4s": ["192.168.1.2"], "ec2.instance_life_cycle": "on-demand", "profiling.agent.config.map_scale_factor": 0, "host.type": "i3.2xlarge", "profiling.host.ip": "192.168.1.2", "profiling.agent.config.bpf_log_level": 0, "profiling.host.sysctl.net.core.bpf_jit_enable": 1, "profiling.agent.config.file": "/etc/prodfiler/prodfiler.conf", "ec2.local_ipv4": "192.168.1.2", "profiling.agent.config.no_kernel_version_check": false, "host.arch": "amd64", "profiling.host.tags": ["cloud_provider:aws", "cloud_environment:qa", "cloud_region:eu-west-1"], "profiling.agent.config.probabilistic_threshold": 100, "profiling.agent.config.disable_tls": false, "profiling.agent.config.tracers": "all", "profiling.agent.start_time": 1700090045589, "profiling.agent.config.max_elements_per_interval": 800, "cloud.provider": "aws", "cloud.region": "eu-west-1", "profiling.agent.config.present_cpu_cores": 8, "profiling.host.kernel_version": "9.9.9-0-aws", "profiling.agent.config.bpf_log_size": 65536, "profiling.agent.config.known_traces_entries": 65536, "profiling.host.sysctl.kernel.unprivileged_bpf_disabled": 1, "profiling.agent.config.verbose": false, "profiling.agent.config.probabilistic_interval": "1m0s", "ec2.placement.availability_zone_id": "euw1-az1", "ec2.security_groups": "", "ec2.local_hostname": "ip-192-168-1-2.eu-west-1.compute.internal", "ec2.placement.availability_zone": "eu-west-1c", "profiling.agent.config.upload_symbols": false, "profiling.host.sysctl.kernel.bpf_stats_enabled": 0, "profiling.host.name": "ip-192-168-1-2", "ec2.mac": "00:11:22:33:44:55", "profiling.host.kernel_proc_version": "Linux version 9.9.9-0-aws", "profiling.agent.config.cache_directory": "/var/cache/optimyze/", "profiling.agent.version": "v8.12.0", "ec2.hostname": "ip-192-168-1-2.eu-west-1.compute.internal", "profiling.agent.config.elastic_mode": false, "ec2.ami_id": "ami-aaaaaaaaaaa", "ec2.instance_id": "i-0b999999999999999" } + - {"profiling.project.id": 100, "host.id": "8457605156473051743", "@timestamp": 1700504426, "ecs.version": "1.12.0", "profiling.agent.build_timestamp": 1688111067, "profiling.instance.private_ipv4s": ["192.168.1.2"], "ec2.instance_life_cycle": "on-demand", "profiling.agent.config.map_scale_factor": 0, "host.type": "i3.2xlarge", "profiling.host.ip": "192.168.1.2", "profiling.agent.config.bpf_log_level": 0, "profiling.host.sysctl.net.core.bpf_jit_enable": 1, "profiling.agent.config.file": "/etc/prodfiler/prodfiler.conf", "ec2.local_ipv4": "192.168.1.2", "profiling.agent.config.no_kernel_version_check": false, "host.arch": "amd64", "profiling.host.tags": ["cloud_provider:aws", "cloud_environment:qa", "cloud_region:eu-west-1"], "profiling.agent.config.probabilistic_threshold": 100, "profiling.agent.config.disable_tls": false, "profiling.agent.config.tracers": "all", "profiling.agent.start_time": 1700090045589, "profiling.agent.config.max_elements_per_interval": 800, "cloud.provider": "aws", "cloud.region": "eu-west-1", "profiling.agent.config.present_cpu_cores": 8, "profiling.host.kernel_version": "9.9.9-0-aws", "profiling.agent.config.bpf_log_size": 65536, "profiling.agent.config.known_traces_entries": 65536, "profiling.host.sysctl.kernel.unprivileged_bpf_disabled": 1, "profiling.agent.config.verbose": false, "profiling.agent.config.probabilistic_interval": "1m0s", "ec2.placement.availability_zone_id": "euw1-az1", "ec2.security_groups": "", "ec2.local_hostname": "ip-192-168-1-2.eu-west-1.compute.internal", "ec2.placement.availability_zone": "eu-west-1c", "profiling.agent.config.upload_symbols": false, "profiling.host.sysctl.kernel.bpf_stats_enabled": 0, "host.name": "ip-192-168-1-2", "ec2.mac": "00:11:22:33:44:55", "profiling.host.kernel_proc_version": "Linux version 9.9.9-0-aws", "profiling.agent.config.cache_directory": "/var/cache/optimyze/", "profiling.agent.version": "v8.12.0", "ec2.hostname": "ip-192-168-1-2.eu-west-1.compute.internal", "profiling.agent.config.elastic_mode": false, "ec2.ami_id": "ami-aaaaaaaaaaa", "ec2.instance_id": "i-0b999999999999999" } - {"index": {"_index": "test-events"}} - {"@timestamp": "1700504427", "events": ["S07KmaoGhvNte78xwwRbZQ"]} --- From 66db8c7d4cafd057dc25fadecdab311ccec6180e Mon Sep 17 00:00:00 2001 From: Dimitris Rempapis Date: Fri, 24 Jan 2025 15:47:44 +0200 Subject: [PATCH 06/23] Test/107515 RestoreTemplateWithMatchOnlyTextMapperIT (#120392) Update the way of comparing stateMaps for equality --- docs/changelog/120392.yaml | 6 ++++++ .../datastreams/DataStreamsSnapshotsIT.java | 14 ++------------ .../org/elasticsearch/test/ESIntegTestCase.java | 9 ++++----- 3 files changed, 12 insertions(+), 17 deletions(-) create mode 100644 docs/changelog/120392.yaml diff --git a/docs/changelog/120392.yaml b/docs/changelog/120392.yaml new file mode 100644 index 0000000000000..69587b4d48241 --- /dev/null +++ b/docs/changelog/120392.yaml @@ -0,0 +1,6 @@ +pr: 120392 +summary: Test/107515 restore template with match only text mapper it fail +area: Search +type: bug +issues: + - 107515 diff --git a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java index e78d9b4f2b8cf..53d75454339a9 100644 --- a/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java +++ b/modules/data-streams/src/internalClusterTest/java/org/elasticsearch/datastreams/DataStreamsSnapshotsIT.java @@ -1137,11 +1137,8 @@ public void testPartialRestoreSnapshotThatIncludesDataStream() { /** * This test is a copy of the {@link #testPartialRestoreSnapshotThatIncludesDataStream()} the only difference - * is that one include the global state and one doesn't. In general this shouldn't matter that's why it used to be - * a random parameter of the test, but because of #107515 it fails when we include the global state. Keep them - * separate until this is fixed. + * is that one include the global state and one doesn't. */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/107515") public void testPartialRestoreSnapshotThatIncludesDataStreamWithGlobalState() { final String snapshot = "test-snapshot"; final String indexWithoutDataStream = "test-idx-no-ds"; @@ -1307,11 +1304,8 @@ public void testExcludeDSFromSnapshotWhenExcludingAnyOfItsIndices() { /** * This test is a copy of the {@link #testExcludeDSFromSnapshotWhenExcludingAnyOfItsIndices()} ()} the only difference - * is that one include the global state and one doesn't. In general this shouldn't matter that's why it used to be - * a random parameter of the test, but because of #107515 it fails when we include the global state. Keep them - * separate until this is fixed. + * is that one include the global state and one doesn't. */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/107515") public void testExcludeDSFromSnapshotWhenExcludingItsIndicesWithGlobalState() { final String snapshot = "test-snapshot"; final String indexWithoutDataStream = "test-idx-no-ds"; @@ -1477,10 +1471,6 @@ public void testWarningHeaderAbsentOnRestoreWithTemplates() throws Exception { } - /** - * This test is muted as it's awaiting the same fix as {@link #testPartialRestoreSnapshotThatIncludesDataStreamWithGlobalState()} - */ - @AwaitsFix(bugUrl = "https://github.com/elastic/elasticsearch/issues/107515") public void testWarningHeaderOnRestoreTemplateFromSnapshot() throws Exception { String datastreamName = "ds"; diff --git a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java index bdfc0a693f7f4..bb259cb9b9788 100644 --- a/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/test/ESIntegTestCase.java @@ -1243,7 +1243,6 @@ protected final void doEnsureClusterStateConsistency(NamedWriteableRegistry name namedWriteableRegistry ); Map masterStateMap = convertToMap(masterClusterState); - int masterClusterStateSize = ClusterState.Builder.toBytes(masterClusterState).length; String masterId = masterClusterState.nodes().getMasterNodeId(); for (SubscribableListener localStateListener : localStates) { localStateListener.andThenAccept(localClusterStateResponse -> { @@ -1255,7 +1254,6 @@ protected final void doEnsureClusterStateConsistency(NamedWriteableRegistry name namedWriteableRegistry ); final Map localStateMap = convertToMap(localClusterState); - final int localClusterStateSize = ClusterState.Builder.toBytes(localClusterState).length; // Check that the non-master node has the same version of the cluster state as the master and // that the master node matches the master (otherwise there is no requirement for the cluster state to // match) @@ -1267,9 +1265,10 @@ protected final void doEnsureClusterStateConsistency(NamedWriteableRegistry name masterClusterState.stateUUID(), localClusterState.stateUUID() ); - // We cannot compare serialization bytes since serialization order of maps is not guaranteed - // but we can compare serialization sizes - they should be the same - assertEquals("cluster state size does not match", masterClusterStateSize, localClusterStateSize); + + // Compare the stateMaps for equality. + assertNull(XContentTestUtils.differenceBetweenMapsIgnoringArrayOrder(masterStateMap, localStateMap)); + // Compare JSON serialization assertNull( "cluster state JSON serialization does not match", From 856a4d7cebc4330e5e27225597275f3d9738bfed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20FOUCRET?= Date: Fri, 24 Jan 2025 14:51:02 +0100 Subject: [PATCH 07/23] LTR - Fix explain failure when index has multiple shards (#120717) --- docs/changelog/120717.yaml | 6 + .../elasticsearch/search/SearchService.java | 54 +++-- .../integration/LearningToRankExplainIT.java | 223 ++++++++++++++++++ .../xpack/ml/LocalStateMachineLearning.java | 10 + 4 files changed, 269 insertions(+), 24 deletions(-) create mode 100644 docs/changelog/120717.yaml create mode 100644 x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/LearningToRankExplainIT.java diff --git a/docs/changelog/120717.yaml b/docs/changelog/120717.yaml new file mode 100644 index 0000000000000..c5609e7e3df5f --- /dev/null +++ b/docs/changelog/120717.yaml @@ -0,0 +1,6 @@ +pr: 120717 +summary: Fix LTR rescorer throws 'local model reference is null' on multi-shards index when explained is enabled +area: Ranking +type: bug +issues: + - 120739 diff --git a/server/src/main/java/org/elasticsearch/search/SearchService.java b/server/src/main/java/org/elasticsearch/search/SearchService.java index 7f3747d321972..efa27b2f3448c 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchService.java +++ b/server/src/main/java/org/elasticsearch/search/SearchService.java @@ -957,32 +957,38 @@ public void executeFetchPhase(ShardFetchRequest request, SearchShardTask task, A final ReaderContext readerContext = findReaderContext(request.contextId(), request); final ShardSearchRequest shardSearchRequest = readerContext.getShardSearchRequest(request.getShardSearchRequest()); final Releasable markAsUsed = readerContext.markAsUsed(getKeepAlive(shardSearchRequest)); - runAsync(getExecutor(readerContext.indexShard()), () -> { - try (SearchContext searchContext = createContext(readerContext, shardSearchRequest, task, ResultsType.FETCH, false)) { - if (request.lastEmittedDoc() != null) { - searchContext.scrollContext().lastEmittedDoc = request.lastEmittedDoc(); - } - searchContext.assignRescoreDocIds(readerContext.getRescoreDocIds(request.getRescoreDocIds())); - searchContext.searcher().setAggregatedDfs(readerContext.getAggregatedDfs(request.getAggregatedDfs())); - try ( - SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor(searchContext, true, System.nanoTime()) - ) { - fetchPhase.execute(searchContext, request.docIds(), request.getRankDocks()); - if (readerContext.singleSession()) { - freeReaderContext(request.contextId()); + rewriteAndFetchShardRequest(readerContext.indexShard(), shardSearchRequest, listener.delegateFailure((l, rewritten) -> { + runAsync(getExecutor(readerContext.indexShard()), () -> { + try (SearchContext searchContext = createContext(readerContext, rewritten, task, ResultsType.FETCH, false)) { + if (request.lastEmittedDoc() != null) { + searchContext.scrollContext().lastEmittedDoc = request.lastEmittedDoc(); } - executor.success(); + searchContext.assignRescoreDocIds(readerContext.getRescoreDocIds(request.getRescoreDocIds())); + searchContext.searcher().setAggregatedDfs(readerContext.getAggregatedDfs(request.getAggregatedDfs())); + try ( + SearchOperationListenerExecutor executor = new SearchOperationListenerExecutor( + searchContext, + true, + System.nanoTime() + ) + ) { + fetchPhase.execute(searchContext, request.docIds(), request.getRankDocks()); + if (readerContext.singleSession()) { + freeReaderContext(request.contextId()); + } + executor.success(); + } + var fetchResult = searchContext.fetchResult(); + // inc-ref fetch result because we close the SearchContext that references it in this try-with-resources block + fetchResult.incRef(); + return fetchResult; + } catch (Exception e) { + assert TransportActions.isShardNotAvailableException(e) == false : new AssertionError(e); + // we handle the failure in the failure listener below + throw e; } - var fetchResult = searchContext.fetchResult(); - // inc-ref fetch result because we close the SearchContext that references it in this try-with-resources block - fetchResult.incRef(); - return fetchResult; - } catch (Exception e) { - assert TransportActions.isShardNotAvailableException(e) == false : new AssertionError(e); - // we handle the failure in the failure listener below - throw e; - } - }, wrapFailureListener(listener, readerContext, markAsUsed)); + }, wrapFailureListener(l, readerContext, markAsUsed)); + })); } protected void checkCancelled(SearchShardTask task) { diff --git a/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/LearningToRankExplainIT.java b/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/LearningToRankExplainIT.java new file mode 100644 index 0000000000000..d05f4a37d5501 --- /dev/null +++ b/x-pack/plugin/ml/src/internalClusterTest/java/org/elasticsearch/xpack/ml/integration/LearningToRankExplainIT.java @@ -0,0 +1,223 @@ +/* + * 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.ml.integration; + +import org.elasticsearch.action.bulk.BulkRequestBuilder; +import org.elasticsearch.action.bulk.BulkResponse; +import org.elasticsearch.action.index.IndexRequest; +import org.elasticsearch.action.support.WriteRequest; +import org.elasticsearch.cluster.metadata.IndexMetadata; +import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.Predicates; +import org.elasticsearch.index.query.QueryBuilders; +import org.elasticsearch.search.SearchHit; +import org.elasticsearch.search.builder.SearchSourceBuilder; +import org.elasticsearch.xcontent.XContentParser; +import org.elasticsearch.xcontent.json.JsonXContent; +import org.elasticsearch.xpack.core.ml.action.PutTrainedModelAction; +import org.elasticsearch.xpack.core.ml.inference.TrainedModelConfig; +import org.elasticsearch.xpack.core.ml.inference.TrainedModelDefinition; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.LearningToRankConfig; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.TargetType; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.ensemble.Ensemble; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.ltr.QueryExtractorBuilder; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.tree.Tree; +import org.elasticsearch.xpack.core.ml.inference.trainedmodel.tree.TreeNode; +import org.elasticsearch.xpack.core.ml.job.config.Operator; +import org.elasticsearch.xpack.core.ml.utils.QueryProvider; +import org.elasticsearch.xpack.ml.support.BaseMlIntegTestCase; +import org.junit.Before; + +import java.io.IOException; +import java.util.Collections; +import java.util.List; + +import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; + +public class LearningToRankExplainIT extends BaseMlIntegTestCase { + + private static final String LTR_SEARCH_INDEX = "ltr-search-index"; + private static final String LTR_MODEL = "ltr-model"; + private static final int NUMBER_OF_NODES = 3; + private static final String DEFAULT_SEARCH_REQUEST_BODY = """ + { + "query": { + "match": { "product": { "query": "TV" } } + }, + "rescore": { + "window_size": 10, + "learning_to_rank": { + "model_id": "ltr-model", + "params": { "keyword": "TV" } + } + } + }"""; + + @Before + public void setupCluster() throws IOException { + internalCluster().ensureAtLeastNumDataNodes(NUMBER_OF_NODES); + ensureStableCluster(); + createLtrModel(); + } + + public void testLtrExplainWithSingleShard() throws IOException { + runLtrExplainTest(1, 1, 2, new float[] { 15f, 11f }); + } + + public void testLtrExplainWithMultipleShards() throws IOException { + runLtrExplainTest(randomIntBetween(2, NUMBER_OF_NODES), 0, 2, new float[] { 15f, 11f }); + } + + public void testLtrExplainWithReplicas() throws IOException { + runLtrExplainTest(1, randomIntBetween(1, NUMBER_OF_NODES - 1), 2, new float[] { 15f, 11f }); + } + + public void testLtrExplainWithMultipleShardsAndReplicas() throws IOException { + runLtrExplainTest(randomIntBetween(2, NUMBER_OF_NODES), randomIntBetween(1, NUMBER_OF_NODES - 1), 2, new float[] { 15f, 11f }); + } + + private void runLtrExplainTest(int numberOfShards, int numberOfReplicas, long expectedTotalHits, float[] expectedScores) + throws IOException { + createLtrIndex(numberOfShards, numberOfReplicas); + try (XContentParser parser = createParser(JsonXContent.jsonXContent, DEFAULT_SEARCH_REQUEST_BODY)) { + assertResponse( + client().prepareSearch(LTR_SEARCH_INDEX) + .setSource(new SearchSourceBuilder().parseXContent(parser, true, Predicates.always())) + .setExplain(true), + searchResponse -> { + assertThat(searchResponse.getHits().getTotalHits().value(), equalTo(expectedTotalHits)); + for (int i = 0; i < expectedScores.length; i++) { + // Check expected score + SearchHit hit = searchResponse.getHits().getHits()[i]; + assertThat(hit.getScore(), equalTo(expectedScores[i])); + + // Check explanation is present and contains the right data + assertThat(hit.getExplanation(), notNullValue()); + assertThat(hit.getExplanation().getValue().floatValue(), equalTo(hit.getScore())); + assertThat(hit.getExplanation().getDescription(), equalTo("rescored using LTR model ltr-model")); + } + } + ); + } + } + + private void createLtrIndex(int numberOfShards, int numberOfReplicas) { + client().admin() + .indices() + .prepareCreate(LTR_SEARCH_INDEX) + .setSettings( + Settings.builder() + .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, numberOfShards) + .put(IndexMetadata.SETTING_NUMBER_OF_REPLICAS, numberOfReplicas) + .build() + ) + .setMapping("product", "type=keyword", "best_seller", "type=boolean") + .get(); + + BulkRequestBuilder bulkRequestBuilder = client().prepareBulk(); + IndexRequest indexRequest = new IndexRequest(LTR_SEARCH_INDEX); + indexRequest.source("product", "TV", "best_seller", true); + bulkRequestBuilder.add(indexRequest); + + indexRequest = new IndexRequest(LTR_SEARCH_INDEX); + indexRequest.source("product", "TV", "best_seller", false); + bulkRequestBuilder.add(indexRequest); + + indexRequest = new IndexRequest(LTR_SEARCH_INDEX); + indexRequest.source("product", "VCR", "best_seller", true); + bulkRequestBuilder.add(indexRequest); + + indexRequest = new IndexRequest(LTR_SEARCH_INDEX); + indexRequest.source("product", "VCR", "best_seller", true); + bulkRequestBuilder.add(indexRequest); + + indexRequest = new IndexRequest(LTR_SEARCH_INDEX); + indexRequest.source("product", "Laptop", "best_seller", true); + bulkRequestBuilder.add(indexRequest); + + BulkResponse bulkResponse = bulkRequestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).get(); + assertThat(bulkResponse.hasFailures(), is(false)); + } + + private void createLtrModel() throws IOException { + client().execute( + PutTrainedModelAction.INSTANCE, + new PutTrainedModelAction.Request( + TrainedModelConfig.builder() + .setModelId(LTR_MODEL) + .setInferenceConfig( + LearningToRankConfig.builder(LearningToRankConfig.EMPTY_PARAMS) + .setLearningToRankFeatureExtractorBuilders( + List.of( + new QueryExtractorBuilder( + "best_seller", + QueryProvider.fromParsedQuery(QueryBuilders.termQuery("best_seller", "true")) + ), + new QueryExtractorBuilder( + "product_match", + QueryProvider.fromParsedQuery(QueryBuilders.termQuery("product", "{{keyword}}")) + ) + ) + ) + .build() + ) + .setParsedDefinition( + new TrainedModelDefinition.Builder().setPreProcessors(Collections.emptyList()) + .setTrainedModel( + Ensemble.builder() + .setFeatureNames(List.of("best_seller", "product_bm25")) + .setTargetType(TargetType.REGRESSION) + .setTrainedModels( + List.of( + Tree.builder() + .setFeatureNames(List.of("best_seller")) + .setTargetType(TargetType.REGRESSION) + .setRoot( + TreeNode.builder(0) + .setSplitFeature(0) + .setSplitGain(12d) + .setThreshold(1d) + .setOperator(Operator.GTE) + .setDefaultLeft(true) + .setLeftChild(1) + .setRightChild(2) + ) + .addLeaf(1, 1) + .addLeaf(2, 5) + .build(), + Tree.builder() + .setFeatureNames(List.of("product_match")) + .setTargetType(TargetType.REGRESSION) + .setRoot( + TreeNode.builder(0) + .setSplitFeature(0) + .setSplitGain(12d) + .setThreshold(1d) + .setOperator(Operator.LT) + .setDefaultLeft(true) + .setLeftChild(1) + .setRightChild(2) + ) + .addLeaf(1, 10) + .addLeaf(2, 1) + .build() + ) + ) + .build() + ) + ) + .validate(true) + .build(), + false + ) + ).actionGet(); + } +} diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/LocalStateMachineLearning.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/LocalStateMachineLearning.java index ff1a1d19779df..426716d0399c5 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/LocalStateMachineLearning.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/LocalStateMachineLearning.java @@ -95,6 +95,16 @@ protected SSLService getSslService() { }); } + @Override + public List> getQueries() { + return mlPlugin.getQueries(); + } + + @Override + public List> getRescorers() { + return mlPlugin.getRescorers(); + } + @Override public List getAggregations() { return mlPlugin.getAggregations(); From 5e662c507e6e123e53546f435cb4b1a08f2d367a Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 24 Jan 2025 09:22:21 -0500 Subject: [PATCH 08/23] Optimize IngestDocMetadata isAvailable (#120753) --- docs/changelog/120753.yaml | 5 +++ .../ingest/common/RenameProcessorTests.java | 43 +++++++++---------- .../elasticsearch/ingest/IngestCtxMap.java | 2 +- .../ingest/IngestDocMetadata.java | 31 ++++++++++--- .../elasticsearch/ingest/IngestService.java | 5 ++- .../java/org/elasticsearch/script/CtxMap.java | 7 ++- .../ingest/IngestCtxMapTests.java | 39 +++++++---------- .../ingest/TestIngestCtxMetadata.java | 27 ------------ .../ingest/TestIngestDocument.java | 39 +++-------------- .../ingest/InferenceProcessorTests.java | 15 ++++--- 10 files changed, 92 insertions(+), 121 deletions(-) create mode 100644 docs/changelog/120753.yaml delete mode 100644 test/framework/src/main/java/org/elasticsearch/ingest/TestIngestCtxMetadata.java diff --git a/docs/changelog/120753.yaml b/docs/changelog/120753.yaml new file mode 100644 index 0000000000000..4885ab4be9add --- /dev/null +++ b/docs/changelog/120753.yaml @@ -0,0 +1,5 @@ +pr: 120753 +summary: Optimize `IngestDocMetadata` `isAvailable` +area: Ingest Node +type: enhancement +issues: [] diff --git a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java index 1ebc5f16a65d3..bc7caf93b0036 100644 --- a/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java +++ b/modules/ingest-common/src/test/java/org/elasticsearch/ingest/common/RenameProcessorTests.java @@ -14,7 +14,6 @@ import org.elasticsearch.ingest.RandomDocumentPicks; import org.elasticsearch.ingest.TestIngestDocument; import org.elasticsearch.ingest.TestTemplateService; -import org.elasticsearch.script.Metadata; import org.elasticsearch.test.ESTestCase; import java.util.ArrayList; @@ -140,42 +139,40 @@ public void testRenameExistingFieldNullValue() throws Exception { public void testRenameAtomicOperationSetFails() throws Exception { Map metadata = new HashMap<>(); - metadata.put("list", List.of("item")); - - IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator( - metadata, - Map.of("new_field", new Metadata.FieldProperty<>(Object.class, true, true, (k, v) -> { - if (v != null) { - throw new UnsupportedOperationException(); - } - }), "list", new Metadata.FieldProperty<>(Object.class, true, true, null)) - ); - Processor processor = createRenameProcessor("list", "new_field", false, false); + metadata.put("_index", "foobar"); + + IngestDocument ingestDocument = TestIngestDocument.withDefaultVersion(metadata); + Processor processor = createRenameProcessor("_index", "_version_type", false, false); try { processor.execute(ingestDocument); fail("processor execute should have failed"); - } catch (UnsupportedOperationException e) { + } catch (IllegalArgumentException e) { // the set failed, the old field has not been removed - assertThat(ingestDocument.getSourceAndMetadata().containsKey("list"), equalTo(true)); - assertThat(ingestDocument.getSourceAndMetadata().containsKey("new_field"), equalTo(false)); + assertThat( + e.getMessage(), + equalTo( + "_version_type must be a null or one of [internal, external, external_gte] " + + "but was [foobar] with type [java.lang.String]" + ) + ); + assertThat(ingestDocument.getSourceAndMetadata().containsKey("_index"), equalTo(true)); + assertThat(ingestDocument.getSourceAndMetadata().containsKey("_version_type"), equalTo(false)); } } public void testRenameAtomicOperationRemoveFails() throws Exception { Map metadata = new HashMap<>(); - metadata.put("list", List.of("item")); + metadata.put("foo", "bar"); - IngestDocument ingestDocument = TestIngestDocument.ofMetadataWithValidator( - metadata, - Map.of("list", new Metadata.FieldProperty<>(Object.class, false, true, null)) - ); - Processor processor = createRenameProcessor("list", "new_field", false, false); + IngestDocument ingestDocument = TestIngestDocument.withDefaultVersion(metadata); + Processor processor = createRenameProcessor("_version", "new_field", false, false); try { processor.execute(ingestDocument); fail("processor execute should have failed"); } catch (IllegalArgumentException e) { - // the set failed, the old field has not been removed - assertThat(ingestDocument.getSourceAndMetadata().containsKey("list"), equalTo(true)); + // the remove failed, the old field has not been removed + assertThat(e.getMessage(), equalTo("_version cannot be removed")); + assertThat(ingestDocument.getSourceAndMetadata().containsKey("_version"), equalTo(true)); assertThat(ingestDocument.getSourceAndMetadata().containsKey("new_field"), equalTo(false)); } } diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestCtxMap.java b/server/src/main/java/org/elasticsearch/ingest/IngestCtxMap.java index d903cc8e52144..a5a1612246a29 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestCtxMap.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestCtxMap.java @@ -29,7 +29,7 @@ * * The map is expected to be used by processors, server code should the typed getter and setters where possible. */ -class IngestCtxMap extends CtxMap { +final class IngestCtxMap extends CtxMap { /** * Create an IngestCtxMap with the given metadata, source and default validators diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocMetadata.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocMetadata.java index 5afeb5079a43f..d7533e26f3df0 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocMetadata.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocMetadata.java @@ -19,7 +19,7 @@ import java.util.Map; import java.util.stream.Collectors; -class IngestDocMetadata extends Metadata { +final class IngestDocMetadata extends Metadata { static final Map> PROPERTIES = Map.of( INDEX, @@ -42,6 +42,17 @@ class IngestDocMetadata extends Metadata { new FieldProperty<>(Map.class).withWritable().withNullable() ); + private static final char UNDERSCORE = '_'; + static { + // there's an optimization here in the overridden isAvailable below, but it only works if the first character of each of these + // keys starts with an underscore, since we know all the keys up front, though, we can just make sure that's always true + for (String key : PROPERTIES.keySet()) { + if (key.charAt(0) != UNDERSCORE) { + throw new IllegalArgumentException("IngestDocMetadata keys must begin with an underscore, but found [" + key + "]"); + } + } + } + protected final ZonedDateTime timestamp; IngestDocMetadata(String index, String id, long version, String routing, VersionType versionType, ZonedDateTime timestamp) { @@ -49,11 +60,7 @@ class IngestDocMetadata extends Metadata { } IngestDocMetadata(Map metadata, ZonedDateTime timestamp) { - this(metadata, PROPERTIES, timestamp); - } - - IngestDocMetadata(Map metadata, Map> properties, ZonedDateTime timestamp) { - super(metadata, properties); + super(metadata, PROPERTIES); this.timestamp = timestamp; } @@ -100,4 +107,16 @@ private static void versionTypeValidator(String key, String value) { + "]" ); } + + @Override + public boolean isAvailable(String key) { + // the key cannot be null or empty because of the nature of the calling code, and this is already validated in IngestDocument + assert key != null && key.isEmpty() == false; + // we can avoid a map lookup on most keys since we know that the only keys that are 'metadata keys' for an ingest document + // must be keys that start with an underscore + if (key.charAt(0) != UNDERSCORE) { + return false; + } + return super.isAvailable(key); + } } diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestService.java b/server/src/main/java/org/elasticsearch/ingest/IngestService.java index 86522742a66c0..b819a1686d23c 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestService.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestService.java @@ -1204,8 +1204,9 @@ private static void updateIndexRequestMetadata(final IndexRequest request, final request.id(metadata.getId()); request.routing(metadata.getRouting()); request.version(metadata.getVersion()); - if (metadata.getVersionType() != null) { - request.versionType(VersionType.fromString(metadata.getVersionType())); + String versionType; + if ((versionType = metadata.getVersionType()) != null) { + request.versionType(VersionType.fromString(versionType)); } Number number; if ((number = metadata.getIfSeqNo()) != null) { diff --git a/server/src/main/java/org/elasticsearch/script/CtxMap.java b/server/src/main/java/org/elasticsearch/script/CtxMap.java index e790eed097f35..1496d70cf39a1 100644 --- a/server/src/main/java/org/elasticsearch/script/CtxMap.java +++ b/server/src/main/java/org/elasticsearch/script/CtxMap.java @@ -14,6 +14,7 @@ import java.util.AbstractCollection; import java.util.AbstractMap; import java.util.AbstractSet; +import java.util.ArrayList; import java.util.Collection; import java.util.HashSet; import java.util.Iterator; @@ -150,10 +151,12 @@ public Object remove(Object key) { @Override public void clear() { // AbstractMap uses entrySet().clear(), it should be quicker to run through the validators, then call the wrapped maps clear - for (String key : metadata.keySet()) { + for (String key : new ArrayList<>(metadata.keySet())) { // copy the key set to get around the ConcurrentModificationException metadata.remove(key); } - // TODO: this is just bogus, there isn't any case where metadata won't trip a failure above? + // note: this is actually bogus in the general case, though! for this to work there must be some Metadata or subclass of Metadata + // for which all the FieldPoperty properties of the metadata are nullable and therefore could have been removed in the previous + // loop -- does such a class even exist? (that is, is there any *real* CtxMap for which the previous loop didn't throw?) source.clear(); } diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java index 2ae9dbfb68599..5a8505e6bb375 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java @@ -110,7 +110,7 @@ public void testRemoveSource() { source.put("abc", 123); source.put("def", 456); source.put("hij", 789); - map = new IngestCtxMap(source, new TestIngestCtxMetadata(new HashMap<>(), new HashMap<>())); + map = new IngestCtxMap(source, new IngestDocMetadata(new HashMap<>(Map.of("_version", 1L)), null)); // Make sure there isn't a ConcurrentModificationException when removing a key from the iterator String removedKey = null; @@ -129,31 +129,18 @@ public void testRemoveSource() { } public void testRemove() { - String cannotRemove = "cannotRemove"; - String canRemove = "canRemove"; - Map metadata = new HashMap<>(); - metadata.put(cannotRemove, "value"); - map = new IngestCtxMap( - new HashMap<>(), - new TestIngestCtxMetadata( - metadata, - Map.of( - cannotRemove, - new Metadata.FieldProperty<>(String.class, false, true, null), - canRemove, - new Metadata.FieldProperty<>(String.class, true, true, null) - ) - ) - ); - String msg = "cannotRemove cannot be removed"; + String cannotRemove = "_version"; // writable, but not *nullable* + String canRemove = "_id"; // writable, and *nullable* + map = new IngestCtxMap(new HashMap<>(), new IngestDocMetadata(new HashMap<>(Map.of(cannotRemove, 1L)), null)); + String msg = "_version cannot be removed"; IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> map.remove(cannotRemove)); assertEquals(msg, err.getMessage()); err = expectThrows(IllegalArgumentException.class, () -> map.put(cannotRemove, null)); - assertEquals("cannotRemove cannot be null", err.getMessage()); + assertEquals("_version cannot be null", err.getMessage()); err = expectThrows(IllegalArgumentException.class, () -> map.entrySet().iterator().next().setValue(null)); - assertEquals("cannotRemove cannot be null", err.getMessage()); + assertEquals("_version cannot be null", err.getMessage()); err = expectThrows(IllegalArgumentException.class, () -> { Iterator> it = map.entrySet().iterator(); @@ -176,6 +163,10 @@ public void testRemove() { err = expectThrows(IllegalArgumentException.class, () -> map.clear()); assertEquals(msg, err.getMessage()); + // depending on iteration order, this may have been removed, so put it back before checking the size + map.put(canRemove, "value"); + assertEquals("value", map.get(canRemove)); + assertEquals(2, map.size()); map.entrySet().remove(new TestEntry(canRemove, "value")); @@ -205,7 +196,7 @@ public void testEntryAndIterator() { source.put("foo", "bar"); source.put("baz", "qux"); source.put("noz", "zon"); - map = new IngestCtxMap(source, TestIngestCtxMetadata.withNullableVersion(metadata)); + map = new IngestCtxMap(source, new IngestDocMetadata(metadata, null)); md = map.getMetadata(); for (Map.Entry entry : map.entrySet()) { @@ -240,8 +231,10 @@ public void testEntryAndIterator() { assertTrue(map.containsKey("noz")); assertEquals(3, map.entrySet().size()); assertEquals(3, map.size()); - map.clear(); - assertEquals(0, map.size()); + + // since an IngestCtxMap must have a _version (and the _version cannot be null), we can't just .clear() + map.entrySet().removeIf(e -> e.getKey().equals("_version") == false); + assertEquals(1, map.size()); } public void testContainsValue() { diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestCtxMetadata.java b/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestCtxMetadata.java deleted file mode 100644 index 7dccecbde6a08..0000000000000 --- a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestCtxMetadata.java +++ /dev/null @@ -1,27 +0,0 @@ -/* - * 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.ingest; - -import org.elasticsearch.script.Metadata; - -import java.util.HashMap; -import java.util.Map; - -public class TestIngestCtxMetadata extends IngestDocMetadata { - public TestIngestCtxMetadata(Map map, Map> properties) { - super(map, Map.copyOf(properties), null); - } - - public static TestIngestCtxMetadata withNullableVersion(Map map) { - Map> updatedProperties = new HashMap<>(IngestDocMetadata.PROPERTIES); - updatedProperties.replace(VERSION, new Metadata.FieldProperty<>(Number.class, true, true, FieldProperty.LONGABLE_NUMBER)); - return new TestIngestCtxMetadata(map, updatedProperties); - } -} diff --git a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java b/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java index 798c824d3be9b..aa1833a659da3 100644 --- a/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java +++ b/test/framework/src/main/java/org/elasticsearch/ingest/TestIngestDocument.java @@ -10,10 +10,8 @@ package org.elasticsearch.ingest; import org.elasticsearch.common.lucene.uid.Versions; -import org.elasticsearch.common.util.Maps; import org.elasticsearch.core.Tuple; import org.elasticsearch.index.VersionType; -import org.elasticsearch.script.Metadata; import org.elasticsearch.test.ESTestCase; import java.util.HashMap; @@ -24,53 +22,30 @@ */ public class TestIngestDocument { public static final long DEFAULT_VERSION = 12345L; - private static String VERSION = IngestDocument.Metadata.VERSION.getFieldName(); - - /** - * Create an IngestDocument for testing that pass an empty mutable map for ingestMetaata - */ - public static IngestDocument withNullableVersion(Map sourceAndMetadata) { - return ofIngestWithNullableVersion(sourceAndMetadata, new HashMap<>()); - } + private static final String VERSION = IngestDocument.Metadata.VERSION.getFieldName(); /** * Create an {@link IngestDocument} from the given sourceAndMetadata and ingestMetadata and a version validator that allows null * _versions. Normally null _version is not allowed, but many tests don't care about that invariant. */ - public static IngestDocument ofIngestWithNullableVersion(Map sourceAndMetadata, Map ingestMetadata) { - Map source = new HashMap<>(sourceAndMetadata); - Map metadata = Maps.newHashMapWithExpectedSize(IngestDocument.Metadata.values().length); - for (IngestDocument.Metadata m : IngestDocument.Metadata.values()) { - String key = m.getFieldName(); - if (sourceAndMetadata.containsKey(key)) { - metadata.put(key, source.remove(key)); - } - } - return new IngestDocument(new IngestCtxMap(source, TestIngestCtxMetadata.withNullableVersion(metadata)), ingestMetadata); - } - - /** - * Create an {@link IngestDocument} with {@link #DEFAULT_VERSION} as the _version metadata, if _version is not already present. - */ - public static IngestDocument withDefaultVersion(Map sourceAndMetadata) { + public static IngestDocument withDefaultVersion(Map sourceAndMetadata, Map ingestMetadata) { if (sourceAndMetadata.containsKey(VERSION) == false) { sourceAndMetadata = new HashMap<>(sourceAndMetadata); sourceAndMetadata.put(VERSION, DEFAULT_VERSION); } - return new IngestDocument(sourceAndMetadata, new HashMap<>()); + return new IngestDocument(sourceAndMetadata, ingestMetadata); } /** - * Create an IngestDocument with a metadata map and validators. The metadata map is passed by reference, not copied, so callers - * can observe changes to the map directly. + * Create an {@link IngestDocument} with {@link #DEFAULT_VERSION} as the _version metadata, if _version is not already present. */ - public static IngestDocument ofMetadataWithValidator(Map metadata, Map> properties) { - return new IngestDocument(new IngestCtxMap(new HashMap<>(), new TestIngestCtxMetadata(metadata, properties)), new HashMap<>()); + public static IngestDocument withDefaultVersion(Map sourceAndMetadata) { + return withDefaultVersion(sourceAndMetadata, new HashMap<>()); } /** * Create an empty ingest document for testing. - * + *

* Adds the required {@code "_version"} metadata key with value {@link #DEFAULT_VERSION}. */ public static IngestDocument emptyIngestDocument() { diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessorTests.java index 6b0b589ace606..248e15f066fdb 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ingest/InferenceProcessorTests.java @@ -298,12 +298,13 @@ public void testGenerateRequestWithEmptyMapping() { Map source = new HashMap<>() { { + put("_version", 345); put("value1", 1); put("value2", 4); put("categorical", "foo"); } }; - IngestDocument document = TestIngestDocument.ofIngestWithNullableVersion(source, new HashMap<>()); + IngestDocument document = TestIngestDocument.withDefaultVersion(source, new HashMap<>()); var request = processor.buildRequest(document); assertThat(request.getObjectsToInfer().get(0), equalTo(source)); @@ -311,7 +312,7 @@ public void testGenerateRequestWithEmptyMapping() { assertEquals(TrainedModelPrefixStrings.PrefixType.INGEST, request.getPrefixType()); Map ingestMetadata = Collections.singletonMap("_value", 3); - document = TestIngestDocument.ofIngestWithNullableVersion(source, ingestMetadata); + document = TestIngestDocument.withDefaultVersion(source, ingestMetadata); Map expected = new HashMap<>(source); expected.put("_ingest", ingestMetadata); @@ -344,12 +345,14 @@ public void testGenerateWithMapping() { ); Map source = Maps.newMapWithExpectedSize(3); + source.put("_version", 234); source.put("value1", 1); source.put("categorical", "foo"); source.put("un_touched", "bar"); - IngestDocument document = TestIngestDocument.withNullableVersion(source); + IngestDocument document = TestIngestDocument.withDefaultVersion(source); Map expectedMap = Maps.newMapWithExpectedSize(5); + expectedMap.put("_version", 234); expectedMap.put("new_value1", 1); expectedMap.put("value1", 1); expectedMap.put("categorical", "foo"); @@ -361,7 +364,7 @@ public void testGenerateWithMapping() { assertEquals(TrainedModelPrefixStrings.PrefixType.INGEST, request.getPrefixType()); Map ingestMetadata = Collections.singletonMap("_value", "baz"); - document = TestIngestDocument.ofIngestWithNullableVersion(source, ingestMetadata); + document = TestIngestDocument.withDefaultVersion(source, ingestMetadata); expectedMap = new HashMap<>(expectedMap); expectedMap.put("metafield", "baz"); expectedMap.put("_ingest", ingestMetadata); @@ -392,12 +395,14 @@ public void testGenerateWithMappingNestedFields() { ); Map source = Maps.newMapWithExpectedSize(3); + source.put("_version", 987); source.put("value1", Collections.singletonMap("foo", 1)); source.put("categorical.bar", "foo"); source.put("un_touched", "bar"); - IngestDocument document = TestIngestDocument.withNullableVersion(source); + IngestDocument document = TestIngestDocument.withDefaultVersion(source); Map expectedMap = Maps.newMapWithExpectedSize(5); + expectedMap.put("_version", 987); expectedMap.put("new_value1", 1); expectedMap.put("value1", Collections.singletonMap("foo", 1)); expectedMap.put("categorical.bar", "foo"); From 39603ec1e297a8a2d0552672b0bd27d2bbae2873 Mon Sep 17 00:00:00 2001 From: Iraklis Psaroudakis Date: Fri, 24 Jan 2025 17:01:01 +0200 Subject: [PATCH 09/23] Reset engine for hollow shards (#120649) Introduces a function in the IndexShard to reset the engine. By default, all Engine implementation will throw in core ES. In stateless, we will extend the prepareForEngineReset() function in order to make a hollow commit or an unhollow commit. Based on the commit, the subsequent new engine will be a hollow or unhollow indexing engine. The reset function takes care to close the engine, which waits for all operations to drain. This, along with the fact we will have blocked ingestion in stateless, and there should be no searches in the indexing tier, should ensure there are no unexpected asynchronous side-effects. Relates ES-10600 --- .../elasticsearch/index/engine/Engine.java | 12 ++++++++ .../elasticsearch/index/shard/IndexShard.java | 29 +++++++++++++++++++ .../index/shard/IndexShardTests.java | 29 ++++++++++++++++++- 3 files changed, 69 insertions(+), 1 deletion(-) diff --git a/server/src/main/java/org/elasticsearch/index/engine/Engine.java b/server/src/main/java/org/elasticsearch/index/engine/Engine.java index 394de0684c104..36fd18144ad6e 100644 --- a/server/src/main/java/org/elasticsearch/index/engine/Engine.java +++ b/server/src/main/java/org/elasticsearch/index/engine/Engine.java @@ -75,6 +75,7 @@ import org.elasticsearch.index.seqno.SequenceNumbers; import org.elasticsearch.index.shard.DenseVectorStats; import org.elasticsearch.index.shard.DocsStats; +import org.elasticsearch.index.shard.IndexShard; import org.elasticsearch.index.shard.ShardId; import org.elasticsearch.index.shard.ShardLongFieldRange; import org.elasticsearch.index.shard.SparseVectorStats; @@ -2334,4 +2335,15 @@ public record FlushResult(boolean flushPerformed, long generation) { public static final long UNKNOWN_GENERATION = -1L; public static final FlushResult NO_FLUSH = new FlushResult(false, UNKNOWN_GENERATION); } + + /** + * Ensures the engine is in a state that it can be closed by a call to {@link IndexShard#resetEngine()}. + * + * In general, resetting the engine should be done with care, to consider any + * in-progress operations and listeners (e.g., primary term and generation listeners). + * At the moment, this is implemented in serverless for a special case that ensures the engine is prepared for reset. + */ + public void prepareForEngineReset() throws IOException { + throw new UnsupportedOperationException("does not support engine reset"); + } } diff --git a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java index ab1c936d1c469..bfa286858f8ba 100644 --- a/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java +++ b/server/src/main/java/org/elasticsearch/index/shard/IndexShard.java @@ -4307,6 +4307,35 @@ public void afterRefresh(boolean didRefresh) { } } + /** + * Reset the current engine to a new one. + * + * Calls {@link Engine#prepareForEngineReset()} on the current engine, then closes it, and loads a new engine without + * doing any translog recovery. + * + * In general, resetting the engine should be done with care, to consider any in-progress operations and listeners. + * At the moment, this is implemented in serverless for a special case that ensures the engine is prepared for reset. + */ + public void resetEngine() { + assert Thread.holdsLock(mutex) == false : "resetting engine under mutex"; + assert waitForEngineOrClosedShardListeners.isDone(); + try { + synchronized (engineMutex) { + final var currentEngine = getEngine(); + currentEngine.prepareForEngineReset(); + var engineConfig = newEngineConfig(replicationTracker); + verifyNotClosed(); + IOUtils.close(currentEngine); + var newEngine = createEngine(engineConfig); + currentEngineReference.set(newEngine); + onNewEngine(newEngine); + } + onSettingsChanged(); + } catch (Exception e) { + failShard("unable to reset engine", e); + } + } + /** * Rollback the current engine to the safe commit, then replay local translog up to the global checkpoint. */ diff --git a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java index 7d436ab5d8d22..4549a329d499a 100644 --- a/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java +++ b/server/src/test/java/org/elasticsearch/index/shard/IndexShardTests.java @@ -4497,7 +4497,7 @@ public void testSupplyTombstoneDoc() throws Exception { closeShards(shard); } - public void testResetEngine() throws Exception { + public void testResetEngineToGlobalCheckpoint() throws Exception { IndexShard shard = newStartedShard(false); indexOnReplicaWithGaps(shard, between(0, 1000), Math.toIntExact(shard.getLocalCheckpoint())); long maxSeqNoBeforeRollback = shard.seqNoStats().getMaxSeqNo(); @@ -4559,6 +4559,33 @@ public void testResetEngine() throws Exception { closeShard(shard, false); } + public void testResetEngine() throws Exception { + var newEngineCreated = new CountDownLatch(2); + var indexShard = newStartedShard(true, Settings.EMPTY, config -> { + try { + return new ReadOnlyEngine(config, null, null, true, Function.identity(), true, true) { + @Override + public void prepareForEngineReset() throws IOException { + ; + } + }; + } finally { + newEngineCreated.countDown(); + } + }); + var newEngineNotification = new CountDownLatch(1); + indexShard.waitForEngineOrClosedShard(ActionListener.running(newEngineNotification::countDown)); + + var onAcquired = new PlainActionFuture(); + indexShard.acquireAllPrimaryOperationsPermits(onAcquired, TimeValue.timeValueMinutes(1L)); + try (var permits = safeGet(onAcquired)) { + indexShard.resetEngine(); + } + safeAwait(newEngineCreated); + safeAwait(newEngineNotification); + closeShard(indexShard, false); + } + /** * This test simulates a scenario seen rarely in ConcurrentSeqNoVersioningIT. Closing a shard while engine is inside * resetEngineToGlobalCheckpoint can lead to check index failure in integration tests. From 484a95043d4dc8303e7b80228a2357d5495951e3 Mon Sep 17 00:00:00 2001 From: Parker Timmins Date: Fri, 24 Jan 2025 09:35:54 -0600 Subject: [PATCH 10/23] Refresh source index before reindexing data stream index (#120752) Add step to the ReindexDatastreamIndexAction which refreshes the source index after setting it to read-only but before calling reindex. Without doing a refresh it is possible for docs from the source index to be missing from the destination index. This happens because the docs arrived before the source index is set to read-only, but because the index hasn't refreshed, the reindex action cannot see these updates. --- docs/changelog/120752.yaml | 6 ++++ muted-tests.yml | 3 -- ...indexDatastreamIndexTransportActionIT.java | 33 ++++++++----------- ...ReindexDataStreamIndexTransportAction.java | 9 +++++ 4 files changed, 29 insertions(+), 22 deletions(-) create mode 100644 docs/changelog/120752.yaml diff --git a/docs/changelog/120752.yaml b/docs/changelog/120752.yaml new file mode 100644 index 0000000000000..674d2190244b1 --- /dev/null +++ b/docs/changelog/120752.yaml @@ -0,0 +1,6 @@ +pr: 120752 +summary: Refresh source index before reindexing data stream index +area: Data streams +type: bug +issues: + - 120314 diff --git a/muted-tests.yml b/muted-tests.yml index 596001b5aac1a..1c58cd98d1e78 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -205,9 +205,6 @@ tests: - class: org.elasticsearch.oldrepos.OldRepositoryAccessIT method: testOldSourceOnlyRepoAccess issue: https://github.com/elastic/elasticsearch/issues/120080 -- class: org.elasticsearch.xpack.migrate.action.ReindexDatastreamIndexTransportActionIT - method: testTsdbStartEndSet - issue: https://github.com/elastic/elasticsearch/issues/120314 - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=snapshot/10_basic/Failed to snapshot indices with synthetic source} issue: https://github.com/elastic/elasticsearch/issues/120332 diff --git a/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java b/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java index cfd4f0901336d..40464d2a43220 100644 --- a/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java +++ b/x-pack/plugin/migrate/src/internalClusterTest/java/org/elasticsearch/xpack/migrate/action/ReindexDatastreamIndexTransportActionIT.java @@ -87,6 +87,7 @@ public void testDestIndexDeletedIfExists() throws Exception { var destIndex = ReindexDataStreamIndexTransportAction.generateDestIndexName(sourceIndex); indicesAdmin().create(new CreateIndexRequest(destIndex)).actionGet(); indexDocs(destIndex, 10); + indicesAdmin().refresh(new RefreshRequest(destIndex)).actionGet(); assertHitCount(prepareSearch(destIndex).setSize(0), 10); // call reindex @@ -195,19 +196,7 @@ public void testMappingsAddedToDestIndex() throws Exception { assumeTrue("requires the migration reindex feature flag", REINDEX_DATA_STREAM_FEATURE_FLAG.isEnabled()); var sourceIndex = randomAlphaOfLength(20).toLowerCase(Locale.ROOT); - String mapping = """ - { - "_doc":{ - "dynamic":"strict", - "properties":{ - "foo1":{ - "type":"text" - } - } - } - } - """; - indicesAdmin().create(new CreateIndexRequest(sourceIndex).mapping(mapping)).actionGet(); + indicesAdmin().create(new CreateIndexRequest(sourceIndex).mapping(MAPPING)).actionGet(); // call reindex var destIndex = client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)) @@ -337,6 +326,12 @@ public void testSettingsAndMappingsFromTemplate() throws IOException { var sourceIndex = "logs-" + randomAlphaOfLength(20).toLowerCase(Locale.ROOT); indicesAdmin().create(new CreateIndexRequest(sourceIndex)).actionGet(); + { + var indexRequest = new IndexRequest(sourceIndex); + indexRequest.source("{ \"foo1\": \"cheese\" }", XContentType.JSON); + client().index(indexRequest).actionGet(); + } + // call reindex var destIndex = client().execute(ReindexDataStreamIndexAction.INSTANCE, new ReindexDataStreamIndexAction.Request(sourceIndex)) .actionGet() @@ -359,6 +354,9 @@ public void testSettingsAndMappingsFromTemplate() throws IOException { // sanity check specific value from dest mapping assertEquals("text", XContentMapValues.extractValue("properties.foo1.type", destMappings)); } + + // verify doc was successfully added + assertHitCount(prepareSearch(destIndex).setSize(0), 1); } private static final String TSDB_MAPPING = """ @@ -455,12 +453,10 @@ public void testTsdbStartEndSet() throws Exception { assertEquals(startTime, destStart); assertEquals(endTime, destEnd); - } - // TODO more logsdb/tsdb specific tests - // TODO more data stream specific tests (how are data streams indices are different from regular indices?) - // TODO check other IndexMetadata fields that need to be fixed after the fact - // TODO what happens if don't have necessary perms for a given index? + // verify doc was successfully added + assertHitCount(prepareSearch(destIndex).setSize(0), 1); + } private static void cleanupMetadataBlocks(String index) { var settings = Settings.builder() @@ -483,7 +479,6 @@ private static void indexDocs(String index, int numDocs) { } BulkResponse bulkResponse = client().bulk(bulkRequest).actionGet(); assertThat(bulkResponse.getItems().length, equalTo(numDocs)); - indicesAdmin().refresh(new RefreshRequest(index)).actionGet(); } private static String formatInstant(Instant instant) { diff --git a/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java b/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java index b915eb3cd3e28..d3fe27006e82e 100644 --- a/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java +++ b/x-pack/plugin/migrate/src/main/java/org/elasticsearch/xpack/migrate/action/ReindexDataStreamIndexTransportAction.java @@ -23,6 +23,7 @@ import org.elasticsearch.action.support.HandledTransportAction; import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.SubscribableListener; +import org.elasticsearch.action.support.broadcast.BroadcastResponse; import org.elasticsearch.action.support.master.AcknowledgedResponse; import org.elasticsearch.client.internal.Client; import org.elasticsearch.cluster.block.ClusterBlockException; @@ -140,6 +141,7 @@ protected void doExecute( } SubscribableListener.newForked(l -> setBlockWrites(sourceIndexName, l, taskId)) + .andThen(l -> refresh(sourceIndexName, l, taskId)) .andThen(l -> deleteDestIfExists(destIndexName, l, taskId)) .andThen(l -> createIndex(sourceIndex, destIndexName, l, taskId)) .andThen(l -> reindex(sourceIndexName, destIndexName, l, taskId)) @@ -175,6 +177,13 @@ public void onFailure(Exception e) { }, parentTaskId); } + private void refresh(String sourceIndexName, ActionListener listener, TaskId parentTaskId) { + logger.debug("Refreshing source index [{}]", sourceIndexName); + var refreshRequest = new RefreshRequest(sourceIndexName); + refreshRequest.setParentTask(parentTaskId); + client.execute(RefreshAction.INSTANCE, refreshRequest, listener); + } + private void deleteDestIfExists(String destIndexName, ActionListener listener, TaskId parentTaskId) { logger.debug("Attempting to delete index [{}]", destIndexName); var deleteIndexRequest = new DeleteIndexRequest(destIndexName).indicesOptions(IGNORE_MISSING_OPTIONS) From 29afce6e003aed9db8067c2720c4e6902dd886ef Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 25 Jan 2025 02:38:47 +1100 Subject: [PATCH 11/23] Mute org.elasticsearch.xpack.ml.integration.LearningToRankExplainIT testLtrExplainWithMultipleShardsAndReplicas #120805 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 1c58cd98d1e78..0843e52cfe390 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -243,6 +243,9 @@ tests: - class: org.elasticsearch.action.search.SearchProgressActionListenerIT method: testSearchProgressWithHits issue: https://github.com/elastic/elasticsearch/issues/120671 +- class: org.elasticsearch.xpack.ml.integration.LearningToRankExplainIT + method: testLtrExplainWithMultipleShardsAndReplicas + issue: https://github.com/elastic/elasticsearch/issues/120805 # Examples: # From 5508d896175b365aa3cfd20cad140a7041985cf2 Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 24 Jan 2025 15:51:25 +0000 Subject: [PATCH 12/23] Use `HARD_CODED_MACHINE_LEARNING_MASTER_NODE_TIMEOUT` in more places (#120701) This hard-coded timeout is trappy, but its removal is not as imminent as `TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT` (see #107984). This commit removes a handful more usages of the trappy timeout bringing us closer to its removal. --- .../org/elasticsearch/xpack/ml/MlInitializationService.java | 2 +- .../xpack/ml/inference/TrainedModelStatsService.java | 5 ++--- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java index 99df16bcd3dc2..a21f59f11540f 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/MlInitializationService.java @@ -155,7 +155,7 @@ public void clusterChanged(ClusterChangedEvent event) { AnnotationIndex.createAnnotationsIndexIfNecessary( client, event.state(), - MasterNodeRequest.TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, + MachineLearning.HARD_CODED_MACHINE_LEARNING_MASTER_NODE_TIMEOUT, ActionListener.wrap(r -> isIndexCreationInProgress.set(false), e -> { if (e.getMessage().equals(previousException)) { logger.debug("Error creating ML annotations index or aliases", e); diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java index 4ee294bcf0d8c..67f2ea74464d0 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/TrainedModelStatsService.java @@ -14,7 +14,6 @@ import org.elasticsearch.action.support.IndicesOptions; import org.elasticsearch.action.support.PlainActionFuture; import org.elasticsearch.action.support.WriteRequest; -import org.elasticsearch.action.support.master.MasterNodeRequest; import org.elasticsearch.action.update.UpdateRequest; import org.elasticsearch.client.internal.OriginSettingClient; import org.elasticsearch.cluster.ClusterChangedEvent; @@ -256,14 +255,14 @@ private void createStatsIndexIfNecessary() { client, clusterState, indexNameExpressionResolver, - MasterNodeRequest.TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, + MachineLearning.HARD_CODED_MACHINE_LEARNING_MASTER_NODE_TIMEOUT, ActionListener.wrap( r -> ElasticsearchMappings.addDocMappingIfMissing( MlStatsIndex.writeAlias(), MlStatsIndex::wrappedMapping, client, clusterState, - MasterNodeRequest.TRAPPY_IMPLICIT_DEFAULT_MASTER_NODE_TIMEOUT, + MachineLearning.HARD_CODED_MACHINE_LEARNING_MASTER_NODE_TIMEOUT, listener, MlStatsIndex.STATS_INDEX_MAPPINGS_VERSION ), From ca83ae53af0721ba93813dd555e9125814cb0661 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:10:50 +1100 Subject: [PATCH 13/23] Mute org.elasticsearch.xpack.test.rest.XPackRestIT test {p0=ml/3rd_party_deployment/Test start deployment fails while model download in progress} #120810 --- muted-tests.yml | 3 +++ 1 file changed, 3 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 0843e52cfe390..269368c5914ba 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -246,6 +246,9 @@ tests: - class: org.elasticsearch.xpack.ml.integration.LearningToRankExplainIT method: testLtrExplainWithMultipleShardsAndReplicas issue: https://github.com/elastic/elasticsearch/issues/120805 +- class: org.elasticsearch.xpack.test.rest.XPackRestIT + method: test {p0=ml/3rd_party_deployment/Test start deployment fails while model download in progress} + issue: https://github.com/elastic/elasticsearch/issues/120810 # Examples: # From 60f78e45a75c66bb8e345aa7e1fc557952e013ab Mon Sep 17 00:00:00 2001 From: David Turner Date: Fri, 24 Jan 2025 16:18:56 +0000 Subject: [PATCH 14/23] Test & document raw transport handshakes (#120785) Updates the `TransportHandshaker` code comments to reflect the new handshake format introduced in #120744, and adds some low-level tests to verify the bytes on the wire are as described in those docs. --- .../transport/TransportHandshaker.java | 40 ++- .../bootstrap/test-framework.policy | 1 + .../TransportHandshakerRawMessageTests.java | 251 ++++++++++++++++++ 3 files changed, 289 insertions(+), 3 deletions(-) create mode 100644 server/src/test/java/org/elasticsearch/transport/TransportHandshakerRawMessageTests.java diff --git a/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java b/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java index a5973e4001444..1a9043d093feb 100644 --- a/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java +++ b/server/src/main/java/org/elasticsearch/transport/TransportHandshaker.java @@ -53,7 +53,7 @@ final class TransportHandshaker { * rely on them matching the real transport protocol (which itself matched the release version numbers), but these days that's no longer * true. * - * Here are some example messages, broken down to show their structure: + * Here are some example messages, broken down to show their structure. See TransportHandshakerRawMessageTests for supporting tests. * * ## v6080099 Request: * @@ -87,7 +87,7 @@ final class TransportHandshaker { * c3 f9 eb 03 -- max acceptable protocol version (vInt: 00000011 11101011 11111001 11000011 == 8060099) * * - * ## v7170099 and v8800000 Requests: + * ## v7170099 Requests: * * 45 53 -- 'ES' marker * 00 00 00 31 -- total message length @@ -106,7 +106,7 @@ final class TransportHandshaker { * 04 -- payload length * c3 f9 eb 03 -- max acceptable protocol version (vInt: 00000011 11101011 11111001 11000011 == 8060099) * - * ## v7170099 and v8800000 Responses: + * ## v7170099 Responses: * * 45 53 -- 'ES' marker * 00 00 00 17 -- total message length @@ -118,6 +118,40 @@ final class TransportHandshaker { * 00 -- no response headers [1] * c3 f9 eb 03 -- max acceptable protocol version (vInt: 00000011 11101011 11111001 11000011 == 8060099) * + * ## v8800000 Requests: + * + * 45 53 -- 'ES' marker + * 00 00 00 36 -- total message length + * 00 00 00 00 00 00 00 01 -- request ID + * 08 -- status flags (0b1000 == handshake request) + * 00 86 47 00 -- handshake protocol version (0x6d6833 == 7170099) + * 00 00 00 19 -- length of variable portion of header + * 00 -- no request headers [1] + * 00 -- no response headers [1] + * 16 -- action string size + * 69 6e 74 65 72 6e 61 6c } + * 3a 74 63 70 2f 68 61 6e }- ASCII representation of HANDSHAKE_ACTION_NAME + * 64 73 68 61 6b 65 } + * 00 -- no parent task ID [3] + * 0a -- payload length + * e8 8f 9b 04 -- requesting node transport version (vInt: 00000100 10011011 10001111 11101000 == 8833000) + * 05 -- requesting node release version string length + * 39 2e 30 2e 30 -- requesting node release version string "9.0.0" + * + * ## v8800000 Responses: + * + * 45 53 -- 'ES' marker + * 00 00 00 1d -- total message length + * 00 00 00 00 00 00 00 01 -- request ID (copied from request) + * 09 -- status flags (0b1001 == handshake response) + * 00 86 47 00 -- handshake protocol version (0x864700 == 8800000, copied from request) + * 00 00 00 02 -- length of following variable portion of header + * 00 -- no request headers [1] + * 00 -- no response headers [1] + * e8 8f 9b 04 -- responding node transport version (vInt: 00000100 10011011 10001111 11101000 == 8833000) + * 05 -- responding node release version string length + * 39 2e 30 2e 30 -- responding node release version string "9.0.0" + * * [1] Thread context headers should be empty; see org.elasticsearch.common.util.concurrent.ThreadContext.ThreadContextStruct.writeTo * for their structure. * [2] A list of strings, which can safely be ignored diff --git a/server/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy b/server/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy index 462fab651c211..77aae99907dfc 100644 --- a/server/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy +++ b/server/src/main/resources/org/elasticsearch/bootstrap/test-framework.policy @@ -127,6 +127,7 @@ grant codeBase "${codebase.netty-transport}" { grant { permission java.net.SocketPermission "127.0.0.1", "accept, connect, resolve"; + permission java.net.SocketPermission "[0:0:0:0:0:0:0:1]", "accept, connect, resolve"; permission java.nio.file.LinkPermission "symbolic"; // needed for keystore tests permission java.lang.RuntimePermission "accessUserInformation"; diff --git a/server/src/test/java/org/elasticsearch/transport/TransportHandshakerRawMessageTests.java b/server/src/test/java/org/elasticsearch/transport/TransportHandshakerRawMessageTests.java new file mode 100644 index 0000000000000..de44ca70f2005 --- /dev/null +++ b/server/src/test/java/org/elasticsearch/transport/TransportHandshakerRawMessageTests.java @@ -0,0 +1,251 @@ +/* + * 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.transport; + +import org.apache.lucene.util.BytesRef; +import org.elasticsearch.Build; +import org.elasticsearch.TransportVersion; +import org.elasticsearch.action.ActionListener; +import org.elasticsearch.cluster.node.DiscoveryNodeUtils; +import org.elasticsearch.common.bytes.BytesArray; +import org.elasticsearch.common.io.stream.BytesStreamOutput; +import org.elasticsearch.common.io.stream.InputStreamStreamInput; +import org.elasticsearch.common.io.stream.OutputStreamStreamOutput; +import org.elasticsearch.common.transport.TransportAddress; +import org.elasticsearch.core.UpdateForV10; +import org.elasticsearch.core.UpdateForV9; +import org.elasticsearch.test.ESSingleNodeTestCase; +import org.elasticsearch.test.TransportVersionUtils; + +import java.net.InetAddress; +import java.net.ServerSocket; +import java.net.Socket; +import java.nio.charset.StandardCharsets; +import java.security.AccessController; +import java.security.PrivilegedExceptionAction; + +import static org.hamcrest.Matchers.allOf; +import static org.hamcrest.Matchers.greaterThan; +import static org.hamcrest.Matchers.greaterThanOrEqualTo; +import static org.hamcrest.Matchers.lessThan; + +public class TransportHandshakerRawMessageTests extends ESSingleNodeTestCase { + + @UpdateForV9(owner = UpdateForV9.Owner.CORE_INFRA) // remove support for v7 handshakes in v9 + public void testV7Handshake() throws Exception { + final BytesRef handshakeRequestBytes; + final var requestId = randomNonNegativeLong(); + try (var outputStream = new BytesStreamOutput()) { + outputStream.setTransportVersion(TransportHandshaker.V7_HANDSHAKE_VERSION); + outputStream.writeLong(requestId); + outputStream.writeByte(TransportStatus.setRequest(TransportStatus.setHandshake((byte) 0))); + outputStream.writeInt(TransportHandshaker.V7_HANDSHAKE_VERSION.id()); + outputStream.writeByte((byte) 0); // no request headers; + outputStream.writeByte((byte) 0); // no response headers; + outputStream.writeStringArray(new String[] { "x-pack" }); // one feature + outputStream.writeString("internal:tcp/handshake"); + outputStream.writeByte((byte) 0); // no parent task ID; + + final var requestNodeTransportVersionId = TransportVersionUtils.randomCompatibleVersion(random()).id(); + assertThat(requestNodeTransportVersionId, allOf(greaterThanOrEqualTo(1 << 22), lessThan(1 << 28))); // 4-byte vInt + outputStream.writeByte((byte) 4); // payload length + outputStream.writeVInt(requestNodeTransportVersionId); + + handshakeRequestBytes = outputStream.bytes().toBytesRef(); + } + + final BytesRef handshakeResponseBytes; + try (var socket = openTransportConnection()) { + var streamOutput = new OutputStreamStreamOutput(socket.getOutputStream()); + streamOutput.write("ES".getBytes(StandardCharsets.US_ASCII)); + streamOutput.writeInt(handshakeRequestBytes.length); + streamOutput.writeBytes(handshakeRequestBytes.bytes, handshakeRequestBytes.offset, handshakeRequestBytes.length); + streamOutput.flush(); + + var streamInput = new InputStreamStreamInput(socket.getInputStream()); + assertEquals((byte) 'E', streamInput.readByte()); + assertEquals((byte) 'S', streamInput.readByte()); + var responseLength = streamInput.readInt(); + handshakeResponseBytes = streamInput.readBytesRef(responseLength); + } + + try (var inputStream = new BytesArray(handshakeResponseBytes).streamInput()) { + assertEquals(requestId, inputStream.readLong()); + assertEquals(TransportStatus.setResponse(TransportStatus.setHandshake((byte) 0)), inputStream.readByte()); + assertEquals(TransportHandshaker.V7_HANDSHAKE_VERSION.id(), inputStream.readInt()); + assertEquals((byte) 0, inputStream.readByte()); // no request headers + assertEquals((byte) 0, inputStream.readByte()); // no response headers + inputStream.setTransportVersion(TransportHandshaker.V7_HANDSHAKE_VERSION); + assertEquals(TransportVersion.current().id(), inputStream.readVInt()); + assertEquals(-1, inputStream.read()); + } + } + + @UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA) // remove support for v8 handshakes in v10 + public void testV8Handshake() throws Exception { + final BytesRef handshakeRequestBytes; + final var requestId = randomNonNegativeLong(); + try (var outputStream = new BytesStreamOutput()) { + outputStream.setTransportVersion(TransportHandshaker.V8_HANDSHAKE_VERSION); + outputStream.writeLong(requestId); + outputStream.writeByte(TransportStatus.setRequest(TransportStatus.setHandshake((byte) 0))); + outputStream.writeInt(TransportHandshaker.V8_HANDSHAKE_VERSION.id()); + outputStream.writeInt(0x1a); // length of variable-length header, always 0x1a + outputStream.writeByte((byte) 0); // no request headers; + outputStream.writeByte((byte) 0); // no response headers; + outputStream.writeByte((byte) 0); // no features; + outputStream.writeString("internal:tcp/handshake"); + outputStream.writeByte((byte) 0); // no parent task ID; + + final var requestNodeTransportVersionId = TransportVersionUtils.randomCompatibleVersion(random()).id(); + assertThat(requestNodeTransportVersionId, allOf(greaterThanOrEqualTo(1 << 22), lessThan(1 << 28))); // 4-byte vInt + outputStream.writeByte((byte) 4); // payload length + outputStream.writeVInt(requestNodeTransportVersionId); + + handshakeRequestBytes = outputStream.bytes().toBytesRef(); + } + + final BytesRef handshakeResponseBytes; + try (var socket = openTransportConnection()) { + var streamOutput = new OutputStreamStreamOutput(socket.getOutputStream()); + streamOutput.write("ES".getBytes(StandardCharsets.US_ASCII)); + streamOutput.writeInt(handshakeRequestBytes.length); + streamOutput.writeBytes(handshakeRequestBytes.bytes, handshakeRequestBytes.offset, handshakeRequestBytes.length); + streamOutput.flush(); + + var streamInput = new InputStreamStreamInput(socket.getInputStream()); + assertEquals((byte) 'E', streamInput.readByte()); + assertEquals((byte) 'S', streamInput.readByte()); + var responseLength = streamInput.readInt(); + handshakeResponseBytes = streamInput.readBytesRef(responseLength); + } + + try (var inputStream = new BytesArray(handshakeResponseBytes).streamInput()) { + assertEquals(requestId, inputStream.readLong()); + assertEquals(TransportStatus.setResponse(TransportStatus.setHandshake((byte) 0)), inputStream.readByte()); + assertEquals(TransportHandshaker.V8_HANDSHAKE_VERSION.id(), inputStream.readInt()); + assertEquals(2, inputStream.readInt()); // length of variable-length header, always 0x02 + assertEquals((byte) 0, inputStream.readByte()); // no request headers + assertEquals((byte) 0, inputStream.readByte()); // no response headers + inputStream.setTransportVersion(TransportHandshaker.V8_HANDSHAKE_VERSION); + assertEquals(TransportVersion.current().id(), inputStream.readVInt()); + assertEquals(-1, inputStream.read()); + } + } + + @UpdateForV10(owner = UpdateForV10.Owner.CORE_INFRA) // remove support for v9 handshakes in v11 + public void testV9Handshake() throws Exception { + final BytesRef handshakeRequestBytes; + final var requestId = randomNonNegativeLong(); + try (var outputStream = new BytesStreamOutput()) { + outputStream.setTransportVersion(TransportHandshaker.V9_HANDSHAKE_VERSION); + outputStream.writeLong(requestId); + outputStream.writeByte(TransportStatus.setRequest(TransportStatus.setHandshake((byte) 0))); + outputStream.writeInt(TransportHandshaker.V9_HANDSHAKE_VERSION.id()); + outputStream.writeInt(0x19); // length of variable-length header, always 0x19 + outputStream.writeByte((byte) 0); // no request headers; + outputStream.writeByte((byte) 0); // no response headers; + outputStream.writeString("internal:tcp/handshake"); + outputStream.writeByte((byte) 0); // no parent task ID; + + final var requestNodeTransportVersionId = TransportVersionUtils.randomCompatibleVersion(random()).id(); + assertThat(requestNodeTransportVersionId, allOf(greaterThanOrEqualTo(1 << 22), lessThan(1 << 28))); // 4-byte vInt + final var releaseVersionLength = between(0, 127 - 5); // so that its length, and the length of the payload, is a one-byte vInt + final var requestNodeReleaseVersion = randomAlphaOfLength(releaseVersionLength); + outputStream.writeByte((byte) (4 + 1 + releaseVersionLength)); // payload length + outputStream.writeVInt(requestNodeTransportVersionId); + outputStream.writeString(requestNodeReleaseVersion); + + handshakeRequestBytes = outputStream.bytes().toBytesRef(); + } + + final BytesRef handshakeResponseBytes; + try (var socket = openTransportConnection()) { + var streamOutput = new OutputStreamStreamOutput(socket.getOutputStream()); + streamOutput.write("ES".getBytes(StandardCharsets.US_ASCII)); + streamOutput.writeInt(handshakeRequestBytes.length); + streamOutput.writeBytes(handshakeRequestBytes.bytes, handshakeRequestBytes.offset, handshakeRequestBytes.length); + streamOutput.flush(); + + var streamInput = new InputStreamStreamInput(socket.getInputStream()); + assertEquals((byte) 'E', streamInput.readByte()); + assertEquals((byte) 'S', streamInput.readByte()); + var responseLength = streamInput.readInt(); + handshakeResponseBytes = streamInput.readBytesRef(responseLength); + } + + try (var inputStream = new BytesArray(handshakeResponseBytes).streamInput()) { + assertEquals(requestId, inputStream.readLong()); + assertEquals(TransportStatus.setResponse(TransportStatus.setHandshake((byte) 0)), inputStream.readByte()); + assertEquals(TransportHandshaker.V9_HANDSHAKE_VERSION.id(), inputStream.readInt()); + assertEquals(2, inputStream.readInt()); // length of variable-length header, always 0x02 + assertEquals((byte) 0, inputStream.readByte()); // no request headers + assertEquals((byte) 0, inputStream.readByte()); // no response headers + inputStream.setTransportVersion(TransportHandshaker.V9_HANDSHAKE_VERSION); + assertEquals(TransportVersion.current().id(), inputStream.readVInt()); + assertEquals(Build.current().version(), inputStream.readString()); + assertEquals(-1, inputStream.read()); + } + } + + public void testOutboundHandshake() throws Exception { + final BytesRef handshakeRequestBytes; + + try (var serverSocket = new ServerSocket(0, 1, InetAddress.getLoopbackAddress())) { + getInstanceFromNode(TransportService.class).openConnection( + DiscoveryNodeUtils.builder(randomIdentifier()) + .address(new TransportAddress(serverSocket.getInetAddress(), serverSocket.getLocalPort())) + .build(), + ConnectionProfile.buildSingleChannelProfile(TransportRequestOptions.Type.REG, null, null, null, null, null), + ActionListener.noop() + ); + + try ( + var acceptedSocket = serverSocket.accept(); + var streamInput = new InputStreamStreamInput(acceptedSocket.getInputStream()) + ) { + assertEquals((byte) 'E', streamInput.readByte()); + assertEquals((byte) 'S', streamInput.readByte()); + var responseLength = streamInput.readInt(); + handshakeRequestBytes = streamInput.readBytesRef(responseLength); + } + } + + final BytesRef payloadBytes; + + try (var inputStream = new BytesArray(handshakeRequestBytes).streamInput()) { + assertThat(inputStream.readLong(), greaterThan(0L)); + assertEquals(TransportStatus.setRequest(TransportStatus.setHandshake((byte) 0)), inputStream.readByte()); + assertEquals(TransportHandshaker.V8_HANDSHAKE_VERSION.id(), inputStream.readInt()); + assertEquals(0x1a, inputStream.readInt()); // length of variable-length header, always 0x1a + assertEquals((byte) 0, inputStream.readByte()); // no request headers + assertEquals((byte) 0, inputStream.readByte()); // no response headers + assertEquals((byte) 0, inputStream.readByte()); // no features + assertEquals("internal:tcp/handshake", inputStream.readString()); + assertEquals((byte) 0, inputStream.readByte()); // no parent task + inputStream.setTransportVersion(TransportHandshaker.V8_HANDSHAKE_VERSION); + payloadBytes = inputStream.readBytesRef(); + assertEquals(-1, inputStream.read()); + } + + try (var inputStream = new BytesArray(payloadBytes).streamInput()) { + inputStream.setTransportVersion(TransportHandshaker.V8_HANDSHAKE_VERSION); + assertEquals(TransportVersion.current().id(), inputStream.readVInt()); + assertEquals(-1, inputStream.read()); + } + } + + private Socket openTransportConnection() throws Exception { + final var transportAddress = randomFrom(getInstanceFromNode(TransportService.class).boundAddress().boundAddresses()).address(); + return AccessController.doPrivileged( + (PrivilegedExceptionAction) (() -> new Socket(transportAddress.getAddress(), transportAddress.getPort())) + ); + } +} From 83dd34f19cb90cca31091c147348a70abcf3762d Mon Sep 17 00:00:00 2001 From: Joe Gallo Date: Fri, 24 Jan 2025 11:39:24 -0500 Subject: [PATCH 15/23] Use getOrDefault in IngestDocument rather than containsKey+get (#120571) --- .../elasticsearch/ingest/IngestDocument.java | 36 +++++++++++-------- .../java/org/elasticsearch/script/CtxMap.java | 12 +++++++ .../org/elasticsearch/script/Metadata.java | 7 ++++ .../ingest/IngestCtxMapTests.java | 12 +++++++ .../org/elasticsearch/script/CtxMapTests.java | 34 ++++++++++++++++++ .../elasticsearch/script/MetadataTests.java | 9 +++++ 6 files changed, 96 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java index 0614e9e92edf2..7982024911beb 100644 --- a/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java +++ b/server/src/main/java/org/elasticsearch/ingest/IngestDocument.java @@ -56,6 +56,9 @@ public final class IngestDocument { // This is the maximum number of nested pipelines that can be within a pipeline. If there are more, we bail out with an error public static final int MAX_PIPELINES = Integer.parseInt(System.getProperty("es.ingest.max_pipelines", "100")); + // a 'not found' sentinel value for use in getOrDefault calls in order to avoid containsKey-and-then-get + private static final Object NOT_FOUND = new Object(); + private final IngestCtxMap ctxMap; private final Map ingestMetadata; @@ -376,11 +379,15 @@ private static ResolveResult resolve(String pathElement, String fullPath, Object if (context == null) { return ResolveResult.error("cannot resolve [" + pathElement + "] from null as part of path [" + fullPath + "]"); } - if (context instanceof Map map) { - if (map.containsKey(pathElement)) { - return ResolveResult.success(map.get(pathElement)); + if (context instanceof Map) { + @SuppressWarnings("unchecked") + Map map = (Map) context; + Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get + if (object == NOT_FOUND) { + return ResolveResult.error("field [" + pathElement + "] not present as part of path [" + fullPath + "]"); + } else { + return ResolveResult.success(object); } - return ResolveResult.error("field [" + pathElement + "] not present as part of path [" + fullPath + "]"); } if (context instanceof List list) { int index; @@ -547,12 +554,13 @@ private void setFieldValue(String path, Object value, boolean append, boolean al if (context instanceof Map) { @SuppressWarnings("unchecked") Map map = (Map) context; - if (map.containsKey(pathElement)) { - context = map.get(pathElement); - } else { - HashMap newMap = new HashMap<>(); + Object object = map.getOrDefault(pathElement, NOT_FOUND); // getOrDefault is faster than containsKey + get + if (object == NOT_FOUND) { + Map newMap = new HashMap<>(); map.put(pathElement, newMap); context = newMap; + } else { + context = object; } } else if (context instanceof List list) { int index; @@ -591,16 +599,16 @@ private void setFieldValue(String path, Object value, boolean append, boolean al @SuppressWarnings("unchecked") Map map = (Map) context; if (append) { - if (map.containsKey(leafKey)) { - Object object = map.get(leafKey); + Object object = map.getOrDefault(leafKey, NOT_FOUND); // getOrDefault is faster than containsKey + get + if (object == NOT_FOUND) { + List list = new ArrayList<>(); + appendValues(list, value); + map.put(leafKey, list); + } else { Object list = appendValues(object, value, allowDuplicates); if (list != object) { map.put(leafKey, list); } - } else { - List list = new ArrayList<>(); - appendValues(list, value); - map.put(leafKey, list); } return; } diff --git a/server/src/main/java/org/elasticsearch/script/CtxMap.java b/server/src/main/java/org/elasticsearch/script/CtxMap.java index 1496d70cf39a1..342e37efcaedf 100644 --- a/server/src/main/java/org/elasticsearch/script/CtxMap.java +++ b/server/src/main/java/org/elasticsearch/script/CtxMap.java @@ -196,6 +196,18 @@ public Object get(Object key) { return directSourceAccess() ? source.get(key) : (SOURCE.equals(key) ? source : null); } + @Override + public Object getOrDefault(Object key, Object defaultValue) { + // uses map directly to avoid Map's implementation that is just get and then containsKey and so could require two isAvailable calls + if (key instanceof String str) { + if (metadata.isAvailable(str)) { + return metadata.getOrDefault(str, defaultValue); + } + return directSourceAccess() ? source.getOrDefault(key, defaultValue) : (SOURCE.equals(key) ? source : defaultValue); + } + return defaultValue; + } + /** * Set of entries of the wrapped map that calls the appropriate validator before changing an entries value or removing an entry. * diff --git a/server/src/main/java/org/elasticsearch/script/Metadata.java b/server/src/main/java/org/elasticsearch/script/Metadata.java index dc5ae51e45af0..fc2a59f7171f2 100644 --- a/server/src/main/java/org/elasticsearch/script/Metadata.java +++ b/server/src/main/java/org/elasticsearch/script/Metadata.java @@ -240,6 +240,13 @@ public Object get(String key) { return map.get(key); } + /** + * Get the value associated with {@param key}, otherwise return {@param defaultValue} + */ + public Object getOrDefault(String key, Object defaultValue) { + return map.getOrDefault(key, defaultValue); + } + /** * Remove the mapping associated with {@param key} * @throws IllegalArgumentException if {@link #isAvailable(String)} is false or the key cannot be removed. diff --git a/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java b/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java index 5a8505e6bb375..33c3ae889040b 100644 --- a/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java +++ b/server/src/test/java/org/elasticsearch/ingest/IngestCtxMapTests.java @@ -329,6 +329,18 @@ public void testHandlesAllVersionTypes() { assertNull(md.getVersionType()); } + public void testGetOrDefault() { + map = new IngestCtxMap(Map.of("foo", "bar"), new IngestDocMetadata(Map.of("_version", 5L), null)); + + // it does the expected thing for fields that are present + assertThat(map.getOrDefault("_version", -1L), equalTo(5L)); + assertThat(map.getOrDefault("foo", "wat"), equalTo("bar")); + + // it does the expected thing for fields that are not present + assertThat(map.getOrDefault("_version_type", "something"), equalTo("something")); + assertThat(map.getOrDefault("baz", "quux"), equalTo("quux")); + } + private static class TestEntry implements Map.Entry { String key; Object value; diff --git a/server/src/test/java/org/elasticsearch/script/CtxMapTests.java b/server/src/test/java/org/elasticsearch/script/CtxMapTests.java index 05c6d22f2d4ec..69b98c940fd9b 100644 --- a/server/src/test/java/org/elasticsearch/script/CtxMapTests.java +++ b/server/src/test/java/org/elasticsearch/script/CtxMapTests.java @@ -17,7 +17,10 @@ import java.util.List; import java.util.Map; +import static org.elasticsearch.script.Metadata.LongField; +import static org.elasticsearch.script.Metadata.VERSION; import static org.hamcrest.Matchers.containsString; +import static org.hamcrest.Matchers.equalTo; public class CtxMapTests extends ESTestCase { CtxMap map; @@ -29,6 +32,37 @@ public void setUp() throws Exception { map = new CtxMap<>(new HashMap<>(), new Metadata(Map.of(), Map.of())); } + @SuppressWarnings("unchecked") + public void testGetOrDefault() { + { + map = new CtxMap<>(Map.of("foo", "bar"), new Metadata(Map.of("_version", 5L), Map.of(VERSION, LongField.withWritable()))); + + // it does the expected thing for fields that are present + assertThat(map.getOrDefault("_version", -1L), equalTo(5L)); + assertThat(((Map) map.getOrDefault("_source", Map.of())).getOrDefault("foo", "wat"), equalTo("bar")); + + // it does the expected thing for fields that are not present + assertThat(map.getOrDefault("_version_type", "something"), equalTo("something")); + assertThat(map.getOrDefault("baz", "quux"), equalTo("quux")); + } + { + map = new CtxMap<>(Map.of("foo", "bar"), new Metadata(Map.of("_version", 5L), Map.of(VERSION, LongField.withWritable()))) { + @Override + protected boolean directSourceAccess() { + return true; + } + }; + + // it does the expected thing for fields that are present + assertThat(map.getOrDefault("_version", -1L), equalTo(5L)); + assertThat(map.getOrDefault("foo", "wat"), equalTo("bar")); + + // it does the expected thing for fields that are not present + assertThat(map.getOrDefault("_version_type", "something"), equalTo("something")); + assertThat(map.getOrDefault("baz", "quux"), equalTo("quux")); + } + } + public void testAddingJunkToCtx() { IllegalArgumentException err = expectThrows(IllegalArgumentException.class, () -> map.put("junk", "stuff")); assertEquals("Cannot put key [junk] with value [stuff] into ctx", err.getMessage()); diff --git a/server/src/test/java/org/elasticsearch/script/MetadataTests.java b/server/src/test/java/org/elasticsearch/script/MetadataTests.java index 80b3dcbf16b66..7830b9d15fdd0 100644 --- a/server/src/test/java/org/elasticsearch/script/MetadataTests.java +++ b/server/src/test/java/org/elasticsearch/script/MetadataTests.java @@ -16,6 +16,8 @@ import java.util.Map; import java.util.Set; +import static org.hamcrest.Matchers.equalTo; + public class MetadataTests extends ESTestCase { Metadata md; private static final Metadata.FieldProperty STRING_PROP = new Metadata.FieldProperty<>(String.class, true, true, null); @@ -279,4 +281,11 @@ public void testImmutablePropertiesMap() { new Metadata(Map.of(), Map.of()); new Metadata(Map.of(), Map.copyOf(new HashMap<>())); } + + public void testGetOrDefault() { + md = new Metadata(new HashMap<>(Map.of("foo", "bar")), Map.of("foo", STRING_PROP, "baz", STRING_PROP)); + assertThat(md.getOrDefault("foo", "wat"), equalTo("bar")); + assertThat(md.getOrDefault("bar", "wat"), equalTo("wat")); + assertThat(md.getOrDefault("yo", "wat"), equalTo("wat")); + } } From 190720d2d89266f8ca033a006d596970d73d75d4 Mon Sep 17 00:00:00 2001 From: Moritz Mack Date: Fri, 24 Jan 2025 17:46:27 +0100 Subject: [PATCH 16/23] Special handling for invalid global labels for APM agent (#120795) --- .../telemetry/apm/internal/APMAgentSettings.java | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java index 9d4822aa9c4d6..68adc97b74449 100644 --- a/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java +++ b/modules/apm/src/main/java/org/elasticsearch/telemetry/apm/internal/APMAgentSettings.java @@ -90,6 +90,11 @@ public void initAgentSystemProperties(Settings settings) { */ @SuppressForbidden(reason = "Need to be able to manipulate APM agent-related properties to set them dynamically") public void setAgentSetting(String key, String value) { + if (key.startsWith("global_labels.")) { + // Invalid agent setting, leftover from flattening global labels in APMJVMOptions + // https://github.com/elastic/elasticsearch/issues/120791 + return; + } final String completeKey = "elastic.apm." + Objects.requireNonNull(key); AccessController.doPrivileged((PrivilegedAction) () -> { if (value == null || value.isEmpty()) { @@ -242,8 +247,8 @@ private static Setting concreteAgentSetting(String namespace, String qua return new Setting<>(qualifiedKey, "", (value) -> { if (qualifiedKey.equals("_na_") == false && PERMITTED_AGENT_KEYS.contains(namespace) == false) { if (namespace.startsWith("global_labels.")) { - // The nested labels syntax is transformed in APMJvmOptions. - // Ignore these here to not fail if not correctly removed. + // Invalid agent setting, leftover from flattening global labels in APMJVMOptions + // https://github.com/elastic/elasticsearch/issues/120791 return value; } throw new IllegalArgumentException("Configuration [" + qualifiedKey + "] is either prohibited or unknown."); From 10e96bde5d6515bc6cfdcb37e619189fb45021c2 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine <58790826+elasticsearchmachine@users.noreply.github.com> Date: Sat, 25 Jan 2025 03:50:24 +1100 Subject: [PATCH 17/23] Mute org.elasticsearch.xpack.test.rest.XPackRestIT org.elasticsearch.xpack.test.rest.XPackRestIT #120816 --- muted-tests.yml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/muted-tests.yml b/muted-tests.yml index 269368c5914ba..d206bc63d7473 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -249,6 +249,8 @@ tests: - class: org.elasticsearch.xpack.test.rest.XPackRestIT method: test {p0=ml/3rd_party_deployment/Test start deployment fails while model download in progress} issue: https://github.com/elastic/elasticsearch/issues/120810 +- class: org.elasticsearch.xpack.test.rest.XPackRestIT + issue: https://github.com/elastic/elasticsearch/issues/120816 # Examples: # From 060c8337e73a4d3d8a4522287c9032dccbedc22d Mon Sep 17 00:00:00 2001 From: Alexander Spies Date: Fri, 24 Jan 2025 18:00:46 +0100 Subject: [PATCH 18/23] ESQL: Simplify TableIdentifier class + rename to IndexPattern (#120797) This class is confusing: - It contains an **unused** `cluster` attribute - we never separate out the cluster, it remains in the `index` field. Also, in the constructor, this field is called `catalog`, which is a concept entirely absent from ESQL at the moment. - It can refer to multiple indices, even multiple wildcard patterns, but doesn't mention this neither in its name nor javadoc. - It has little to do with tables, which is likely a remnant of this class' usage in SQL, before the `esql.core` split. This PR removes the `cluster` attribute, renames the class to `IndexPattern`, and adds javadoc to clarify that it can also contain stuff like `remote1:idx1,remote-*:idx-*`. --- .../xpack/esql/analysis/Analyzer.java | 14 ++-- .../xpack/esql/analysis/PreAnalyzer.java | 2 +- .../xpack/esql/analysis/TableInfo.java | 8 +- .../xpack/esql/parser/LogicalPlanBuilder.java | 8 +- .../xpack/esql/plan/IndexPattern.java | 57 +++++++++++++++ .../xpack/esql/plan/TableIdentifier.java | 73 ------------------- .../esql/plan/logical/UnresolvedRelation.java | 24 +++--- .../xpack/esql/session/EsqlSession.java | 27 ++++--- .../esql/session/EsqlSessionCCSUtils.java | 2 +- .../elasticsearch/xpack/esql/CsvTests.java | 2 +- .../xpack/esql/analysis/AnalyzerTests.java | 4 +- .../parser/AbstractStatementParserTests.java | 4 +- .../esql/parser/StatementParserTests.java | 20 ++--- .../session/EsqlSessionCCSUtilsTests.java | 10 +-- 14 files changed, 120 insertions(+), 135 deletions(-) create mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/IndexPattern.java delete mode 100644 x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/TableIdentifier.java diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java index 552e90e0e90f9..4f5ff35b84054 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Analyzer.java @@ -64,7 +64,7 @@ import org.elasticsearch.xpack.esql.index.EsIndex; import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.parser.ParsingException; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Drop; import org.elasticsearch.xpack.esql.plan.logical.Enrich; @@ -202,7 +202,9 @@ private static class ResolveTable extends ParameterizedAnalyzerRule { List list = p.indexMode() == IndexMode.LOOKUP ? lookupIndices : indices; - list.add(new TableInfo(p.table())); + list.add(new TableInfo(p.indexPattern())); }); plan.forEachUp(Enrich.class, unresolvedEnriches::add); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/TableInfo.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/TableInfo.java index eff658e8997b0..38d368bd2bfad 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/TableInfo.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/TableInfo.java @@ -7,17 +7,17 @@ package org.elasticsearch.xpack.esql.analysis; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; public class TableInfo { - private final TableIdentifier id; + private final IndexPattern id; - public TableInfo(TableIdentifier id) { + public TableInfo(IndexPattern id) { this.id = id; } - public TableIdentifier id() { + public IndexPattern id() { return id; } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java index e7e3527f6b4aa..ba74bf467f2aa 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/parser/LogicalPlanBuilder.java @@ -34,7 +34,7 @@ import org.elasticsearch.xpack.esql.expression.Order; import org.elasticsearch.xpack.esql.expression.UnresolvedNamePattern; import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Dissect; import org.elasticsearch.xpack.esql.plan.logical.Drop; @@ -255,7 +255,7 @@ public LogicalPlan visitRowCommand(EsqlBaseParser.RowCommandContext ctx) { @Override public LogicalPlan visitFromCommand(EsqlBaseParser.FromCommandContext ctx) { Source source = source(ctx); - TableIdentifier table = new TableIdentifier(source, null, visitIndexPattern(ctx.indexPattern())); + IndexPattern table = new IndexPattern(source, visitIndexPattern(ctx.indexPattern())); Map metadataMap = new LinkedHashMap<>(); if (ctx.metadata() != null) { for (var c : ctx.metadata().UNQUOTED_SOURCE()) { @@ -468,7 +468,7 @@ public LogicalPlan visitMetricsCommand(EsqlBaseParser.MetricsCommandContext ctx) throw new IllegalArgumentException("METRICS command currently requires a snapshot build"); } Source source = source(ctx); - TableIdentifier table = new TableIdentifier(source, null, visitIndexPattern(ctx.indexPattern())); + IndexPattern table = new IndexPattern(source, visitIndexPattern(ctx.indexPattern())); if (ctx.aggregates == null && ctx.grouping == null) { return new UnresolvedRelation(source, table, false, List.of(), IndexMode.STANDARD, null, "METRICS"); @@ -530,7 +530,7 @@ public PlanFactory visitJoinCommand(EsqlBaseParser.JoinCommandContext ctx) { UnresolvedRelation right = new UnresolvedRelation( source(target), - new TableIdentifier(source(target.index), null, rightPattern), + new IndexPattern(source(target.index), rightPattern), false, emptyList(), IndexMode.LOOKUP, diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/IndexPattern.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/IndexPattern.java new file mode 100644 index 0000000000000..fdaac1c1cc64c --- /dev/null +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/IndexPattern.java @@ -0,0 +1,57 @@ +/* + * 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.plan; + +import org.elasticsearch.xpack.esql.core.tree.Source; + +import java.util.Objects; + +/** + * Contains an index pattern together with its {@link Source}. Can also be a comma-separated list, like {@code idx-*,remote:other-idx*}. + */ +public class IndexPattern { + + private final Source source; + private final String indexPattern; + + public IndexPattern(Source source, String indexPattern) { + this.source = source; + this.indexPattern = indexPattern; + } + + public String indexPattern() { + return indexPattern; + } + + @Override + public int hashCode() { + return Objects.hash(indexPattern); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + + if (obj == null || getClass() != obj.getClass()) { + return false; + } + + IndexPattern other = (IndexPattern) obj; + return Objects.equals(indexPattern, other.indexPattern); + } + + public Source source() { + return source; + } + + @Override + public String toString() { + return indexPattern; + } +} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/TableIdentifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/TableIdentifier.java deleted file mode 100644 index 532d93eec48af..0000000000000 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/TableIdentifier.java +++ /dev/null @@ -1,73 +0,0 @@ -/* - * 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.plan; - -import org.elasticsearch.xpack.esql.core.tree.Source; - -import java.util.Objects; - -import static org.elasticsearch.transport.RemoteClusterAware.REMOTE_CLUSTER_INDEX_SEPARATOR; - -public class TableIdentifier { - - private final Source source; - - private final String cluster; - private final String index; - - public TableIdentifier(Source source, String catalog, String index) { - this.source = source; - this.cluster = catalog; - this.index = index; - } - - public String cluster() { - return cluster; - } - - public String index() { - return index; - } - - @Override - public int hashCode() { - return Objects.hash(cluster, index); - } - - @Override - public boolean equals(Object obj) { - if (this == obj) { - return true; - } - - if (obj == null || getClass() != obj.getClass()) { - return false; - } - - TableIdentifier other = (TableIdentifier) obj; - return Objects.equals(index, other.index) && Objects.equals(cluster, other.cluster); - } - - public Source source() { - return source; - } - - public String qualifiedIndex() { - return cluster != null ? cluster + REMOTE_CLUSTER_INDEX_SEPARATOR + index : index; - } - - @Override - public String toString() { - StringBuilder builder = new StringBuilder(); - if (cluster != null) { - builder.append(cluster); - builder.append(REMOTE_CLUSTER_INDEX_SEPARATOR); - } - builder.append(index); - return builder.toString(); - } -} diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnresolvedRelation.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnresolvedRelation.java index 384c3f7a340ae..0a20e1dd9080d 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnresolvedRelation.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plan/logical/UnresolvedRelation.java @@ -12,7 +12,7 @@ import org.elasticsearch.xpack.esql.core.expression.Attribute; import org.elasticsearch.xpack.esql.core.tree.NodeInfo; import org.elasticsearch.xpack.esql.core.tree.Source; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import java.util.Collections; import java.util.List; @@ -22,7 +22,7 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable { - private final TableIdentifier table; + private final IndexPattern indexPattern; private final boolean frozen; private final List metadataFields; /* @@ -40,7 +40,7 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable { public UnresolvedRelation( Source source, - TableIdentifier table, + IndexPattern indexPattern, boolean frozen, List metadataFields, IndexMode indexMode, @@ -48,11 +48,11 @@ public UnresolvedRelation( String commandName ) { super(source); - this.table = table; + this.indexPattern = indexPattern; this.frozen = frozen; this.metadataFields = metadataFields; this.indexMode = indexMode; - this.unresolvedMsg = unresolvedMessage == null ? "Unknown index [" + table.index() + "]" : unresolvedMessage; + this.unresolvedMsg = unresolvedMessage == null ? "Unknown index [" + indexPattern.indexPattern() + "]" : unresolvedMessage; this.commandName = commandName; } @@ -68,11 +68,11 @@ public String getWriteableName() { @Override protected NodeInfo info() { - return NodeInfo.create(this, UnresolvedRelation::new, table, frozen, metadataFields, indexMode, unresolvedMsg, commandName); + return NodeInfo.create(this, UnresolvedRelation::new, indexPattern, frozen, metadataFields, indexMode, unresolvedMsg, commandName); } - public TableIdentifier table() { - return table; + public IndexPattern indexPattern() { + return indexPattern; } public boolean frozen() { @@ -124,7 +124,7 @@ public String unresolvedMessage() { @Override public int hashCode() { - return Objects.hash(source(), table, metadataFields, indexMode, unresolvedMsg); + return Objects.hash(source(), indexPattern, metadataFields, indexMode, unresolvedMsg); } @Override @@ -138,7 +138,7 @@ public boolean equals(Object obj) { } UnresolvedRelation other = (UnresolvedRelation) obj; - return Objects.equals(table, other.table) + return Objects.equals(indexPattern, other.indexPattern) && Objects.equals(frozen, other.frozen) && Objects.equals(metadataFields, other.metadataFields) && indexMode == other.indexMode @@ -147,11 +147,11 @@ public boolean equals(Object obj) { @Override public List nodeProperties() { - return singletonList(table); + return singletonList(indexPattern); } @Override public String toString() { - return UNRESOLVED_PREFIX + table.index(); + return UNRESOLVED_PREFIX + indexPattern.indexPattern(); } } diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java index b10f766babb36..b40e49df49c84 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java @@ -56,7 +56,7 @@ import org.elasticsearch.xpack.esql.optimizer.PhysicalPlanOptimizer; import org.elasticsearch.xpack.esql.parser.EsqlParser; import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.Keep; @@ -321,7 +321,9 @@ public void analyzedPlan( EsqlSessionCCSUtils.checkForCcsLicense(executionInfo, indices, indicesExpressionGrouper, verifier.licenseState()); final Set targetClusters = enrichPolicyResolver.groupIndicesPerCluster( - indices.stream().flatMap(t -> Arrays.stream(Strings.commaDelimitedListToStringArray(t.id().index()))).toArray(String[]::new) + indices.stream() + .flatMap(t -> Arrays.stream(Strings.commaDelimitedListToStringArray(t.id().indexPattern()))) + .toArray(String[]::new) ).keySet(); var listener = SubscribableListener.newForked( @@ -373,14 +375,14 @@ public void analyzedPlan( } private void preAnalyzeLookupIndex(TableInfo tableInfo, PreAnalysisResult result, ActionListener listener) { - TableIdentifier table = tableInfo.id(); - Set fieldNames = result.wildcardJoinIndices().contains(table.index()) ? IndexResolver.ALL_FIELDS : result.fieldNames; + IndexPattern table = tableInfo.id(); + Set fieldNames = result.wildcardJoinIndices().contains(table.indexPattern()) ? IndexResolver.ALL_FIELDS : result.fieldNames; // call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types indexResolver.resolveAsMergedMapping( - table.index(), + table.indexPattern(), fieldNames, null, - listener.map(indexResolution -> result.addLookupIndexResolution(table.index(), indexResolution)) + listener.map(indexResolution -> result.addLookupIndexResolution(table.indexPattern(), indexResolution)) ); // TODO: Verify that the resolved index actually has indexMode: "lookup" } @@ -400,9 +402,12 @@ private void preAnalyzeIndices( // known to be unavailable from the enrich policy API call Map unavailableClusters = result.enrichResolution.getUnavailableClusters(); TableInfo tableInfo = indices.get(0); - TableIdentifier table = tableInfo.id(); + IndexPattern table = tableInfo.id(); - Map clusterIndices = indicesExpressionGrouper.groupIndices(IndicesOptions.DEFAULT, table.index()); + Map clusterIndices = indicesExpressionGrouper.groupIndices( + IndicesOptions.DEFAULT, + table.indexPattern() + ); for (Map.Entry entry : clusterIndices.entrySet()) { final String clusterAlias = entry.getKey(); String indexExpr = Strings.arrayToCommaDelimitedString(entry.getValue().indices()); @@ -431,7 +436,9 @@ private void preAnalyzeIndices( String indexExpressionToResolve = EsqlSessionCCSUtils.createIndexExpressionFromAvailableClusters(executionInfo); if (indexExpressionToResolve.isEmpty()) { // if this was a pure remote CCS request (no local indices) and all remotes are offline, return an empty IndexResolution - listener.onResponse(result.withIndexResolution(IndexResolution.valid(new EsIndex(table.index(), Map.of(), Map.of())))); + listener.onResponse( + result.withIndexResolution(IndexResolution.valid(new EsIndex(table.indexPattern(), Map.of(), Map.of()))) + ); } else { // call the EsqlResolveFieldsAction (field-caps) to resolve indices and get field types indexResolver.resolveAsMergedMapping( @@ -588,7 +595,7 @@ static PreAnalysisResult fieldNames(LogicalPlan parsed, Set enrichPolicy } if (keepCommandReferences.isEmpty()) { // No KEEP commands after the JOIN, so we need to mark this index for "*" field resolution - wildcardJoinIndices.add(((UnresolvedRelation) join.right()).table().index()); + wildcardJoinIndices.add(((UnresolvedRelation) join.right()).indexPattern().indexPattern()); } else { // Keep commands can reference the join columns with names that shadow aliases, so we block their removal keepJoinReferences.addAll(keepCommandReferences); diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java index 304a54741d44b..6be243456e040 100644 --- a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java +++ b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtils.java @@ -311,7 +311,7 @@ public static void checkForCcsLicense( for (TableInfo tableInfo : indices) { Map groupedIndices; try { - groupedIndices = indicesGrouper.groupIndices(IndicesOptions.DEFAULT, tableInfo.id().index()); + groupedIndices = indicesGrouper.groupIndices(IndicesOptions.DEFAULT, tableInfo.id().indexPattern()); } catch (NoSuchRemoteClusterException e) { if (EsqlLicenseChecker.isCcsAllowed(licenseState)) { throw e; diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java index 89150d6a52534..ae9c12fd7c711 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java @@ -462,7 +462,7 @@ private static CsvTestsDataLoader.MultiIndexTestDataset testDatasets(LogicalPlan throw new IllegalArgumentException("unexpected index resolution to multiple entries [" + preAnalysis.indices.size() + "]"); } - String indexName = indices.get(0).id().index(); + String indexName = indices.get(0).id().indexPattern(); List datasets = new ArrayList<>(); if (indexName.endsWith("*")) { String indexPrefix = indexName.substring(0, indexName.length() - 1); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java index 48366282e4e10..b01a82819e2ea 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/AnalyzerTests.java @@ -41,7 +41,7 @@ import org.elasticsearch.xpack.esql.index.IndexResolution; import org.elasticsearch.xpack.esql.parser.ParsingException; import org.elasticsearch.xpack.esql.parser.QueryParams; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Enrich; import org.elasticsearch.xpack.esql.plan.logical.EsRelation; @@ -98,7 +98,7 @@ public class AnalyzerTests extends ESTestCase { private static final UnresolvedRelation UNRESOLVED_RELATION = new UnresolvedRelation( EMPTY, - new TableIdentifier(EMPTY, null, "idx"), + new IndexPattern(EMPTY, "idx"), false, List.of(), IndexMode.STANDARD, diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java index 31ea4f2712b98..111c553d34a0e 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/AbstractStatementParserTests.java @@ -17,7 +17,7 @@ import org.elasticsearch.xpack.esql.core.expression.UnresolvedAttribute; import org.elasticsearch.xpack.esql.core.type.DataType; import org.elasticsearch.xpack.esql.expression.function.UnresolvedFunction; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan; import org.elasticsearch.xpack.esql.plan.logical.UnresolvedRelation; @@ -72,7 +72,7 @@ static UnresolvedFunction function(String name, List args) { } static UnresolvedRelation relation(String name) { - return new UnresolvedRelation(EMPTY, new TableIdentifier(EMPTY, null, name), false, List.of(), IndexMode.STANDARD, null, "FROM"); + return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, name), false, List.of(), IndexMode.STANDARD, null, "FROM"); } static Literal integer(int i) { diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java index ac41c7b0f52bc..792b43433e1ee 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/parser/StatementParserTests.java @@ -42,7 +42,7 @@ import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.GreaterThanOrEqual; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThan; import org.elasticsearch.xpack.esql.expression.predicate.operator.comparison.LessThanOrEqual; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.plan.logical.Aggregate; import org.elasticsearch.xpack.esql.plan.logical.Dissect; import org.elasticsearch.xpack.esql.plan.logical.Drop; @@ -2052,7 +2052,7 @@ private void assertStringAsIndexPattern(String string, String statement) { LogicalPlan from = statement(statement); assertThat(from, instanceOf(UnresolvedRelation.class)); UnresolvedRelation table = (UnresolvedRelation) from; - assertThat(table.table().index(), is(string)); + assertThat(table.indexPattern().indexPattern(), is(string)); } private void assertStringAsLookupIndexPattern(String string, String statement) { @@ -2283,20 +2283,12 @@ public void testInvalidAlias() { } private LogicalPlan unresolvedRelation(String index) { - return new UnresolvedRelation(EMPTY, new TableIdentifier(EMPTY, null, index), false, List.of(), IndexMode.STANDARD, null, "FROM"); + return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, index), false, List.of(), IndexMode.STANDARD, null, "FROM"); } private LogicalPlan unresolvedTSRelation(String index) { List metadata = List.of(new MetadataAttribute(EMPTY, MetadataAttribute.TSID_FIELD, DataType.KEYWORD, false)); - return new UnresolvedRelation( - EMPTY, - new TableIdentifier(EMPTY, null, index), - false, - metadata, - IndexMode.TIME_SERIES, - null, - "FROM TS" - ); + return new UnresolvedRelation(EMPTY, new IndexPattern(EMPTY, index), false, metadata, IndexMode.TIME_SERIES, null, "FROM TS"); } public void testMetricWithGroupKeyAsAgg() { @@ -2956,8 +2948,8 @@ public void testValidJoinPattern() { var plan = statement("FROM " + basePattern + " | " + type + " JOIN " + joinPattern + " ON " + onField); var join = as(plan, LookupJoin.class); - assertThat(as(join.left(), UnresolvedRelation.class).table().index(), equalTo(unquoteIndexPattern(basePattern))); - assertThat(as(join.right(), UnresolvedRelation.class).table().index(), equalTo(unquoteIndexPattern(joinPattern))); + assertThat(as(join.left(), UnresolvedRelation.class).indexPattern().indexPattern(), equalTo(unquoteIndexPattern(basePattern))); + assertThat(as(join.right(), UnresolvedRelation.class).indexPattern().indexPattern(), equalTo(unquoteIndexPattern(joinPattern))); var joinType = as(join.config().type(), JoinTypes.UsingJoinType.class); assertThat(joinType.columns(), hasSize(1)); diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java index 05d04ff1315e6..a84e5b144e64c 100644 --- a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java +++ b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/session/EsqlSessionCCSUtilsTests.java @@ -32,7 +32,7 @@ import org.elasticsearch.xpack.esql.core.type.EsField; import org.elasticsearch.xpack.esql.index.EsIndex; import org.elasticsearch.xpack.esql.index.IndexResolution; -import org.elasticsearch.xpack.esql.plan.TableIdentifier; +import org.elasticsearch.xpack.esql.plan.IndexPattern; import org.elasticsearch.xpack.esql.type.EsFieldTests; import java.util.ArrayList; @@ -702,7 +702,7 @@ public void testCheckForCcsLicense() { // local only search does not require an enterprise license { List indices = new ArrayList<>(); - indices.add(new TableInfo(new TableIdentifier(EMPTY, null, randomFrom("idx", "idx1,idx2*")))); + indices.add(new TableInfo(new IndexPattern(EMPTY, randomFrom("idx", "idx1,idx2*")))); checkForCcsLicense(executionInfo, indices, indicesGrouper, enterpriseLicenseValid); checkForCcsLicense(executionInfo, indices, indicesGrouper, platinumLicenseValid); @@ -727,10 +727,10 @@ public void testCheckForCcsLicense() { List indices = new ArrayList<>(); final String indexExprWithRemotes = randomFrom("remote:idx", "idx1,remote:idx2*,remote:logs,c*:idx4"); if (randomBoolean()) { - indices.add(new TableInfo(new TableIdentifier(EMPTY, null, indexExprWithRemotes))); + indices.add(new TableInfo(new IndexPattern(EMPTY, indexExprWithRemotes))); } else { - indices.add(new TableInfo(new TableIdentifier(EMPTY, null, randomFrom("idx", "idx1,idx2*")))); - indices.add(new TableInfo(new TableIdentifier(EMPTY, null, indexExprWithRemotes))); + indices.add(new TableInfo(new IndexPattern(EMPTY, randomFrom("idx", "idx1,idx2*")))); + indices.add(new TableInfo(new IndexPattern(EMPTY, indexExprWithRemotes))); } // licenses that work From ae7f3b6931d09cd0ae48e1b7f03cd9c5795ee628 Mon Sep 17 00:00:00 2001 From: Pawan Kartik Date: Fri, 24 Jan 2025 17:09:21 +0000 Subject: [PATCH 19/23] Fix and unmute `CrossClusterEsqlRCS2UnavailableRemotesIT` tests (#120402) * Fix and unmute `CrossClusterEsqlRCS2UnavailableRemotesIT` tests * [CI] Auto commit changes from spotless * Fix test --------- Co-authored-by: elasticsearchmachine --- muted-tests.yml | 3 --- ...rossClusterEsqlRCS2UnavailableRemotesIT.java | 17 ++++++++++++++--- 2 files changed, 14 insertions(+), 6 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index d206bc63d7473..06ed24e068e4f 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -131,9 +131,6 @@ tests: - class: org.elasticsearch.xpack.ml.integration.RegressionIT method: testTwoJobsWithSameRandomizeSeedUseSameTrainingSet issue: https://github.com/elastic/elasticsearch/issues/117805 -- class: org.elasticsearch.xpack.remotecluster.CrossClusterEsqlRCS2UnavailableRemotesIT - method: testEsqlRcs2UnavailableRemoteScenarios - issue: https://github.com/elastic/elasticsearch/issues/117419 - class: org.elasticsearch.xpack.esql.action.EsqlActionTaskIT method: testCancelRequestWhenFailingFetchingPages issue: https://github.com/elastic/elasticsearch/issues/118193 diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS2UnavailableRemotesIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS2UnavailableRemotesIT.java index b62d82c47f753..50bd386b2172c 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS2UnavailableRemotesIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS2UnavailableRemotesIT.java @@ -25,10 +25,12 @@ import java.util.Map; import java.util.concurrent.atomic.AtomicReference; +import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.greaterThan; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.oneOf; public class CrossClusterEsqlRCS2UnavailableRemotesIT extends AbstractRemoteClusterSecurityTestCase { private static final AtomicReference> API_KEY_MAP_REF = new AtomicReference<>(); @@ -179,8 +181,10 @@ private void remoteClusterShutdownWithSkipUnavailableTrue() throws Exception { Map failuresMap = (Map) remoteClusterFailures.get(0); Map reason = (Map) failuresMap.get("reason"); - assertThat(reason.get("type").toString(), equalTo("connect_transport_exception")); - assertThat(reason.get("reason").toString(), containsString("Unable to connect to [my_remote_cluster]")); + assertThat( + reason.get("type").toString(), + oneOf("node_disconnected_exception", "connect_transport_exception", "node_not_connected_exception") + ); } finally { fulfillingCluster.start(); closeFulfillingClusterClient(); @@ -201,7 +205,14 @@ private void remoteClusterShutdownWithSkipUnavailableFalse() throws Exception { // A simple query that targets our remote cluster. String query = "FROM *,my_remote_cluster:* | LIMIT 10"; ResponseException ex = expectThrows(ResponseException.class, () -> performRequestWithRemoteSearchUser(esqlRequest(query))); - assertThat(ex.getMessage(), containsString("connect_transport_exception")); + assertThat( + ex.getMessage(), + anyOf( + containsString("node_disconnected_exception"), + containsString("connect_transport_exception"), + containsString("node_not_connected_exception") + ) + ); } finally { fulfillingCluster.start(); closeFulfillingClusterClient(); From 05513550e7150d17827b04d5d69ccebdd2360c13 Mon Sep 17 00:00:00 2001 From: Pawan Kartik Date: Fri, 24 Jan 2025 17:11:33 +0000 Subject: [PATCH 20/23] Fix and unmute `CrossClusterEsqlRCS1UnavailableRemotesIT` tests (#120388) * Unmute `CrossClusterEsqlRCS1UnavailableRemotesIT.testEsqlRcs1UnavailableRemoteScenarios` * Track `node_not_connected_exception` --- muted-tests.yml | 3 --- .../CrossClusterEsqlRCS1UnavailableRemotesIT.java | 10 +++++++++- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/muted-tests.yml b/muted-tests.yml index 06ed24e068e4f..dba2e055e7351 100644 --- a/muted-tests.yml +++ b/muted-tests.yml @@ -146,9 +146,6 @@ tests: - class: org.elasticsearch.action.search.SearchQueryThenFetchAsyncActionTests method: testBottomFieldSort issue: https://github.com/elastic/elasticsearch/issues/118214 -- class: org.elasticsearch.xpack.remotecluster.CrossClusterEsqlRCS1UnavailableRemotesIT - method: testEsqlRcs1UnavailableRemoteScenarios - issue: https://github.com/elastic/elasticsearch/issues/118350 - class: org.elasticsearch.xpack.searchablesnapshots.RetrySearchIntegTests method: testSearcherId issue: https://github.com/elastic/elasticsearch/issues/118374 diff --git a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1UnavailableRemotesIT.java b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1UnavailableRemotesIT.java index c7623779ee214..9759b1440c3e8 100644 --- a/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1UnavailableRemotesIT.java +++ b/x-pack/plugin/security/qa/multi-cluster/src/javaRestTest/java/org/elasticsearch/xpack/remotecluster/CrossClusterEsqlRCS1UnavailableRemotesIT.java @@ -26,6 +26,7 @@ import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.is; +import static org.hamcrest.Matchers.anyOf; import static org.hamcrest.Matchers.greaterThan; public class CrossClusterEsqlRCS1UnavailableRemotesIT extends AbstractRemoteClusterSecurityTestCase { @@ -178,7 +179,14 @@ private void remoteClusterShutdownWithSkipUnavailableFalse() throws Exception { // A simple query that targets our remote cluster. String query = "FROM *,my_remote_cluster:* | LIMIT 10"; ResponseException ex = expectThrows(ResponseException.class, () -> client().performRequest(esqlRequest(query))); - assertThat(ex.getMessage(), containsString("connect_transport_exception")); + assertThat( + ex.getMessage(), + anyOf( + containsString("connect_transport_exception"), + containsString("node_disconnected_exception"), + containsString("node_not_connected_exception") + ) + ); } finally { fulfillingCluster.start(); closeFulfillingClusterClient(); From 969cd70aa00f0c104b5b94dabee05ac84be0e0d9 Mon Sep 17 00:00:00 2001 From: Kostas Krikellas <131142368+kkrik-es@users.noreply.github.com> Date: Fri, 24 Jan 2025 19:36:59 +0200 Subject: [PATCH 21/23] Restore source matching in randomized logsdb tests (#120773) Applies the fix in `SourceMatcher` from #120756, along with disabling `SCALED_FLOAT` and `HALF_FLOAT` that have accuracy issues leading to false positives. --- .../org/elasticsearch/index/mapper/DocumentParser.java | 8 -------- .../index/mapper/IgnoredSourceFieldMapperTests.java | 4 +--- .../elasticsearch/logsdb/datageneration/FieldType.java | 8 +------- .../datasource/DefaultMappingParametersHandler.java | 3 +-- .../xpack/logsdb/qa/matchers/source/SourceMatcher.java | 3 +-- 5 files changed, 4 insertions(+), 22 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java index 9ddb6f0d496a0..5a417c541d716 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/DocumentParser.java @@ -729,28 +729,20 @@ private static void parseNonDynamicArray( XContentParser parser = context.parser(); XContentParser.Token token; - int elements = 0; while ((token = parser.nextToken()) != XContentParser.Token.END_ARRAY) { if (token == XContentParser.Token.START_OBJECT) { - elements = Integer.MAX_VALUE; parseObject(context, lastFieldName); } else if (token == XContentParser.Token.START_ARRAY) { - elements = Integer.MAX_VALUE; parseArray(context, lastFieldName); } else if (token == XContentParser.Token.VALUE_NULL) { - elements++; parseNullValue(context, lastFieldName); } else if (token == null) { throwEOFOnParseArray(arrayFieldName, context); } else { assert token.isValue(); - elements++; parseValue(context, lastFieldName); } } - if (elements <= 1 && canRemoveSingleLeafElement) { - context.removeLastIgnoredField(fullPath); - } postProcessDynamicArrayMapping(context, lastFieldName); } diff --git a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java index 14902aa419b9f..2b36c0ce0b5a4 100644 --- a/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java +++ b/server/src/test/java/org/elasticsearch/index/mapper/IgnoredSourceFieldMapperTests.java @@ -743,9 +743,7 @@ public void testIndexStoredArraySourceSingleLeafElement() throws IOException { b.startObject("int_value").field("type", "integer").endObject(); })).documentMapper(); var syntheticSource = syntheticSource(documentMapper, b -> b.array("int_value", new int[] { 10 })); - assertEquals("{\"int_value\":10}", syntheticSource); - ParsedDocument doc = documentMapper.parse(source(syntheticSource)); - assertNull(doc.rootDoc().getField("_ignored_source")); + assertEquals("{\"int_value\":[10]}", syntheticSource); } public void testIndexStoredArraySourceSingleLeafElementAndNull() throws IOException { diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/FieldType.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/FieldType.java index 96b75f29382e2..07744851aba3e 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/FieldType.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/FieldType.java @@ -13,11 +13,9 @@ import org.elasticsearch.logsdb.datageneration.fields.leaf.ByteFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.DoubleFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.FloatFieldDataGenerator; -import org.elasticsearch.logsdb.datageneration.fields.leaf.HalfFloatFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.IntegerFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.KeywordFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.LongFieldDataGenerator; -import org.elasticsearch.logsdb.datageneration.fields.leaf.ScaledFloatFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.ShortFieldDataGenerator; import org.elasticsearch.logsdb.datageneration.fields.leaf.UnsignedLongFieldDataGenerator; @@ -32,9 +30,7 @@ public enum FieldType { SHORT("short"), BYTE("byte"), DOUBLE("double"), - FLOAT("float"), - HALF_FLOAT("half_float"), - SCALED_FLOAT("scaled_float"); + FLOAT("float"); private final String name; @@ -52,8 +48,6 @@ public FieldDataGenerator generator(String fieldName, DataSource dataSource) { case BYTE -> new ByteFieldDataGenerator(fieldName, dataSource); case DOUBLE -> new DoubleFieldDataGenerator(fieldName, dataSource); case FLOAT -> new FloatFieldDataGenerator(fieldName, dataSource); - case HALF_FLOAT -> new HalfFloatFieldDataGenerator(fieldName, dataSource); - case SCALED_FLOAT -> new ScaledFloatFieldDataGenerator(fieldName, dataSource); }; } diff --git a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java index b639108ea6ad2..db13867fe71ad 100644 --- a/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java +++ b/test/framework/src/main/java/org/elasticsearch/logsdb/datageneration/datasource/DefaultMappingParametersHandler.java @@ -32,8 +32,7 @@ public DataSourceResponse.LeafMappingParametersGenerator handle(DataSourceReques return new DataSourceResponse.LeafMappingParametersGenerator(switch (request.fieldType()) { case KEYWORD -> keywordMapping(request, map); - case LONG, INTEGER, SHORT, BYTE, DOUBLE, FLOAT, HALF_FLOAT, UNSIGNED_LONG -> plain(map); - case SCALED_FLOAT -> scaledFloatMapping(map); + case LONG, INTEGER, SHORT, BYTE, DOUBLE, FLOAT, UNSIGNED_LONG -> plain(map); }); } diff --git a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/qa/matchers/source/SourceMatcher.java b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/qa/matchers/source/SourceMatcher.java index cd2bb361d065d..d4d53a85c6e88 100644 --- a/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/qa/matchers/source/SourceMatcher.java +++ b/x-pack/plugin/logsdb/src/javaRestTest/java/org/elasticsearch/xpack/logsdb/qa/matchers/source/SourceMatcher.java @@ -104,7 +104,7 @@ private MatchResult compareSource(Map> actual, Map> actual, Map Date: Fri, 24 Jan 2025 17:44:40 +0000 Subject: [PATCH 22/23] Remove support for types field in watcher search (#120748) In 8.x, setting the `input.search.request.types` field in the payload when creating a watcher to an empty array was allowed, although it resulted in a deprecation warning and had no effect (and any value other than an empty array would result in an error). In 9.x, support for this field is entirely removed, and the empty array will also result in an error. We have already introduced a script to be run as part of the upgrade which removes the field from existing watches in https://github.com/elastic/elasticsearch/pull/120371. This also removes an unrelated TODO in passing, because we are not going to do that (the functionality it refers to exists and is not deprecated so cannot be removed). ES-9747 #close #comment Types in search request removed in https://github.com/elastic/elasticsearch/pull/120748 --- docs/changelog/120748.yaml | 15 +++++++++++++++ .../search/WatcherSearchTemplateRequest.java | 14 -------------- .../search/WatcherSearchTemplateRequestTests.java | 10 ++++++---- 3 files changed, 21 insertions(+), 18 deletions(-) create mode 100644 docs/changelog/120748.yaml diff --git a/docs/changelog/120748.yaml b/docs/changelog/120748.yaml new file mode 100644 index 0000000000000..e2ec312f189b0 --- /dev/null +++ b/docs/changelog/120748.yaml @@ -0,0 +1,15 @@ +pr: 120748 +summary: Removing support for types field in watcher search +area: Watcher +type: breaking +issues: [] +breaking: + title: Removing support for types field in watcher search + area: REST API + details: >- + Previously, setting the `input.search.request.types` field in the payload when creating a watcher to an empty array + was allowed, although it resulted in a deprecation warning and had no effect (and any value other than an empty + array would result in an error). Now, support for this field is entirely removed, and the empty array will also + result in an error. + impact: Users should stop setting this field (which did not have any effect anyway). + notable: false diff --git a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java index 15208f86a5e2b..f62830f2345ea 100644 --- a/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java +++ b/x-pack/plugin/watcher/src/main/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequest.java @@ -12,7 +12,6 @@ import org.elasticsearch.common.Strings; import org.elasticsearch.common.bytes.BytesArray; import org.elasticsearch.common.bytes.BytesReference; -import org.elasticsearch.common.logging.DeprecationCategory; import org.elasticsearch.common.logging.DeprecationLogger; import org.elasticsearch.core.Nullable; import org.elasticsearch.script.Script; @@ -173,7 +172,6 @@ public static WatcherSearchTemplateRequest fromXContent(XContentParser parser, S IndicesOptions indicesOptions = DEFAULT_INDICES_OPTIONS; BytesReference searchSource = null; Script template = null; - // TODO this is to retain BWC compatibility in 7.0 and can be removed for 8.0 boolean totalHitsAsInt = true; XContentParser.Token token; @@ -196,17 +194,6 @@ public static WatcherSearchTemplateRequest fromXContent(XContentParser parser, S ); } } - } else if (TYPES_FIELD.match(currentFieldName, parser.getDeprecationHandler())) { - // Tolerate an empty types array, because some watches created internally in 6.x have - // an empty types array in their search, and it's clearly equivalent to typeless. - if (parser.nextToken() != XContentParser.Token.END_ARRAY) { - throw new ElasticsearchParseException( - "could not read search request. unsupported non-empty array field [" + currentFieldName + "]" - ); - } - // Empty types arrays still generate the same deprecation warning they did in 7.x. - // Ideally they should be removed from the definition. - deprecationLogger.critical(DeprecationCategory.PARSING, "watcher_search_input", TYPES_DEPRECATION_MESSAGE); } else { throw new ElasticsearchParseException( "could not read search request. unexpected array field [" + currentFieldName + "]" @@ -289,7 +276,6 @@ public int hashCode() { } private static final ParseField INDICES_FIELD = new ParseField("indices"); - private static final ParseField TYPES_FIELD = new ParseField("types"); private static final ParseField BODY_FIELD = new ParseField("body"); private static final ParseField SEARCH_TYPE_FIELD = new ParseField("search_type"); private static final ParseField INDICES_OPTIONS_FIELD = new ParseField("indices_options"); diff --git a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java index 620580ee09824..96d8201b37a15 100644 --- a/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java +++ b/x-pack/plugin/watcher/src/test/java/org/elasticsearch/xpack/watcher/support/search/WatcherSearchTemplateRequestTests.java @@ -17,7 +17,6 @@ import java.util.Map; import static java.util.Collections.singletonMap; -import static org.hamcrest.Matchers.arrayContaining; import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.is; @@ -52,8 +51,11 @@ public void testFromXContentWithEmptyTypes() throws IOException { """; try (XContentParser parser = createParser(JsonXContent.jsonXContent, source)) { parser.nextToken(); - WatcherSearchTemplateRequest result = WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values())); - assertThat(result.getIndices(), arrayContaining(".ml-anomalies-*")); + ElasticsearchParseException e = expectThrows( + ElasticsearchParseException.class, + () -> WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values())) + ); + assertThat(e.getMessage(), is("could not read search request. unexpected array field [types]")); } } @@ -74,7 +76,7 @@ public void testFromXContentWithNonEmptyTypes() throws IOException { ElasticsearchParseException.class, () -> WatcherSearchTemplateRequest.fromXContent(parser, randomFrom(SearchType.values())) ); - assertThat(e.getMessage(), is("could not read search request. unsupported non-empty array field [types]")); + assertThat(e.getMessage(), is("could not read search request. unexpected array field [types]")); } } From 4783d1f991633dbc3b7397fe13c63b6b7d70dc37 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Aur=C3=A9lien=20FOUCRET?= Date: Fri, 24 Jan 2025 18:56:04 +0100 Subject: [PATCH 23/23] LTR sometines throw NullPointerException: Cannot read field "approximation" because "top" is null (#120809) * Add check on the DisiPriorityQueue size. * Update docs/changelog/120809.yaml * Add a unit test. --- docs/changelog/120809.yaml | 6 ++++ .../inference/ltr/QueryFeatureExtractor.java | 7 ++++- .../ltr/QueryFeatureExtractorTests.java | 28 +++++++++++++++++++ 3 files changed, 40 insertions(+), 1 deletion(-) create mode 100644 docs/changelog/120809.yaml diff --git a/docs/changelog/120809.yaml b/docs/changelog/120809.yaml new file mode 100644 index 0000000000000..30a3736dc93a4 --- /dev/null +++ b/docs/changelog/120809.yaml @@ -0,0 +1,6 @@ +pr: 120809 +summary: LTR sometines throw `NullPointerException:` Cannot read field "approximation" + because "top" is null +area: Ranking +type: bug +issues: [] diff --git a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java index bbc377a67ec0b..08c141c0858ca 100644 --- a/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java +++ b/x-pack/plugin/ml/src/main/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractor.java @@ -55,11 +55,16 @@ public void setNextReader(LeafReaderContext segmentContext) throws IOException { } scorers.add(scorer); } - rankerIterator = new DisjunctionDISIApproximation(disiPriorityQueue); + + rankerIterator = disiPriorityQueue.size() > 0 ? new DisjunctionDISIApproximation(disiPriorityQueue) : null; } @Override public void addFeatures(Map featureMap, int docId) throws IOException { + if (rankerIterator == null) { + return; + } + rankerIterator.advance(docId); for (int i = 0; i < featureNames.size(); i++) { Scorer scorer = scorers.get(i); diff --git a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java index 3878ce5dab087..3b25a266bf412 100644 --- a/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java +++ b/x-pack/plugin/ml/src/test/java/org/elasticsearch/xpack/ml/inference/ltr/QueryFeatureExtractorTests.java @@ -23,6 +23,7 @@ import org.elasticsearch.index.query.QueryRewriteContext; import org.elasticsearch.index.query.SearchExecutionContext; import org.elasticsearch.test.AbstractBuilderTestCase; +import org.elasticsearch.test.ESTestCase; import org.elasticsearch.xpack.core.ml.inference.trainedmodel.ltr.QueryExtractorBuilder; import org.elasticsearch.xpack.core.ml.utils.QueryProvider; @@ -31,12 +32,14 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.stream.Stream; import static org.hamcrest.Matchers.anEmptyMap; import static org.hamcrest.Matchers.hasEntry; import static org.hamcrest.Matchers.hasKey; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.not; +import static org.mockito.Mockito.mock; public class QueryFeatureExtractorTests extends AbstractBuilderTestCase { @@ -125,4 +128,29 @@ public void testQueryExtractor() throws IOException { dir.close(); } + public void testEmptyDisiPriorityQueue() throws IOException { + addDocs( + new String[] { "the quick brown fox", "the slow brown fox", "the grey dog", "yet another string" }, + new int[] { 5, 10, 12, 11 } + ); + + // Scorers returned by weights are null + List featureNames = randomList(1, 10, ESTestCase::randomIdentifier); + List weights = Stream.generate(() -> mock(Weight.class)).limit(featureNames.size()).toList(); + + QueryFeatureExtractor featureExtractor = new QueryFeatureExtractor(featureNames, weights); + + for (LeafReaderContext leafReaderContext : searcher.getLeafContexts()) { + int maxDoc = leafReaderContext.reader().maxDoc(); + featureExtractor.setNextReader(leafReaderContext); + for (int i = 0; i < maxDoc; i++) { + Map featureMap = new HashMap<>(); + featureExtractor.addFeatures(featureMap, i); + assertThat(featureMap, anEmptyMap()); + } + } + + reader.close(); + dir.close(); + } }