From dafbf90f298c09a6fd439bfc707045cb8a2dbbf1 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 11:33:44 +0200 Subject: [PATCH 01/36] Add timeout to SynonymsManagementAPIService put synonyms --- .../SynonymsManagementAPIServiceIT.java | 34 ++++++---- .../org/elasticsearch/TransportVersions.java | 1 + .../action/synonyms/PutSynonymRuleAction.java | 39 +++++++++--- .../TransportDeleteSynonymRuleAction.java | 4 +- .../TransportDeleteSynonymsAction.java | 2 +- .../TransportPutSynonymRuleAction.java | 5 +- .../indices/store/IndicesStore.java | 17 +++++ .../synonyms/RestPutSynonymRuleAction.java | 1 + .../SynonymsManagementAPIService.java | 63 ++++++++++++++----- 9 files changed, 126 insertions(+), 40 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index a7b98b4639130..fea567857ea89 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -23,6 +23,7 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; +import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.action.synonyms.SynonymsTestUtils.randomSynonymRule; import static org.elasticsearch.action.synonyms.SynonymsTestUtils.randomSynonymsSet; import static org.mockito.ArgumentMatchers.anyString; @@ -51,8 +52,12 @@ public void testCreateManySynonyms() throws Exception { CountDownLatch putLatch = new CountDownLatch(1); String synonymSetId = randomIdentifier(); int rulesNumber = randomIntBetween(maxSynonymSets / 2, maxSynonymSets); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(rulesNumber, rulesNumber), new ActionListener<>() { - @Override + synonymsManagementAPIService.putSynonymsSet( + synonymSetId, + randomSynonymsSet(rulesNumber, rulesNumber), + DEFAULT_TIMEOUT, + new ActionListener<>() { + @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { assertEquals( SynonymsManagementAPIService.UpdateSynonymsResultStatus.CREATED, @@ -95,6 +100,7 @@ public void testCreateTooManySynonymsAtOnce() throws InterruptedException { synonymsManagementAPIService.putSynonymsSet( randomIdentifier(), randomSynonymsSet(maxSynonymSets + 1, maxSynonymSets * 2), + DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { @@ -120,8 +126,12 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept int rulesToUpdate = randomIntBetween(1, 10); int synonymsToCreate = maxSynonymSets - rulesToUpdate; String synonymSetId = randomIdentifier(); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(synonymsToCreate), new ActionListener<>() { - @Override + synonymsManagementAPIService.putSynonymsSet( + synonymSetId, + randomSynonymsSet(synonymsToCreate), + DEFAULT_TIMEOUT, + new ActionListener<>() { + @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Create as many rules as should fail SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); @@ -137,7 +147,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { fail(e); } - }); + }, DEFAULT_TIMEOUT); } try { updatedRulesLatch.await(5, TimeUnit.SECONDS); @@ -166,8 +176,8 @@ public void onFailure(Exception e) { } updatedRulesLatch.countDown(); } - } - ); + }, + DEFAULT_TIMEOUT); } try { insertRulesLatch.await(5, TimeUnit.SECONDS); @@ -189,7 +199,7 @@ public void testUpdateRuleWithMaxSynonyms() throws InterruptedException { CountDownLatch latch = new CountDownLatch(1); String synonymSetId = randomIdentifier(); SynonymRule[] synonymsSet = randomSynonymsSet(maxSynonymSets, maxSynonymSets); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, new ActionListener<>() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails @@ -206,8 +216,8 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { fail("Should update a rule that already exists at max capcity"); } - } - ); + }, + DEFAULT_TIMEOUT); } @Override @@ -224,7 +234,7 @@ public void testCreateRuleWithMaxSynonyms() throws InterruptedException { String synonymSetId = randomIdentifier(); String ruleId = randomIdentifier(); SynonymRule[] synonymsSet = randomSynonymsSet(maxSynonymSets, maxSynonymSets); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, new ActionListener<>() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails @@ -238,7 +248,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { latch.countDown(); } - }); + }, DEFAULT_TIMEOUT); } @Override diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index 00943d04275dd..c10784bfec842 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -214,6 +214,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_REMOVE_AGGREGATE_TYPE = def(9_045_0_00); public static final TransportVersion ADD_PROJECT_ID_TO_DSL_ERROR_INFO = def(9_046_0_00); public static final TransportVersion SEMANTIC_TEXT_CHUNKING_CONFIG = def(9_047_00_0); + public static final TransportVersion SYNONYMS_UPDATE_TIMEOUT = def(9_048_0_00); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java index 6d0d24dbfee05..2c4abb39d1fb0 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; @@ -34,14 +35,16 @@ public class PutSynonymRuleAction extends ActionType { public static final PutSynonymRuleAction INSTANCE = new PutSynonymRuleAction(); public static final String NAME = "cluster:admin/synonym_rules/put"; + public static final int DEFAULT_TIMEOUT = 5; + public PutSynonymRuleAction() { super(NAME); } public static class Request extends ActionRequest { private final String synonymsSetId; - private final SynonymRule synonymRule; + private final int timeout; public static final ParseField SYNONYMS_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( @@ -58,20 +61,28 @@ public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRule = new SynonymRule(in); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { + this.timeout = in.readInt(); + } else { + this.timeout = DEFAULT_TIMEOUT; + } } - public Request(String synonymsSetId, String synonymRuleId, BytesReference content, XContentType contentType) throws IOException { + public Request(String synonymsSetId, String synonymRuleId, int timeout, BytesReference content, XContentType contentType) + throws IOException { this.synonymsSetId = synonymsSetId; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { this.synonymRule = PARSER.apply(parser, synonymRuleId); } catch (Exception e) { throw new IllegalArgumentException("Failed to parse: " + content.utf8ToString(), e); } + this.timeout = timeout; } - Request(String synonymsSetId, SynonymRule synonymRule) { + Request(String synonymsSetId, SynonymRule synonymRule, int timeout) { this.synonymsSetId = synonymsSetId; this.synonymRule = synonymRule; + this.timeout = timeout; } @Override @@ -83,6 +94,9 @@ public ActionRequestValidationException validate() { if (Strings.isNullOrEmpty(synonymRule.id())) { validationException = ValidateActions.addValidationError("synonym rule id must be specified", validationException); } + if (timeout < 0) { + validationException = ValidateActions.addValidationError("timeout must be greater than or equal to 0", validationException); + } String error = synonymRule.validate(); if (error != null) { validationException = ValidateActions.addValidationError(error, validationException); @@ -96,6 +110,9 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); synonymRule.writeTo(out); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { + out.writeInt(timeout); + } } public String synonymsSetId() { @@ -106,17 +123,23 @@ public SynonymRule synonymRule() { return synonymRule; } + public int timeout() { + return timeout; + } + @Override public boolean equals(Object o) { - if (this == o) return true; - if (o == null || getClass() != o.getClass()) return false; - Request request = (Request) o; - return Objects.equals(synonymsSetId, request.synonymsSetId) && Objects.equals(synonymRule, request.synonymRule); + if (o instanceof Request request) { + return timeout == request.timeout + && Objects.equals(synonymsSetId, request.synonymsSetId) + && Objects.equals(synonymRule, request.synonymRule); + } + return false; } @Override public int hashCode() { - return Objects.hash(synonymsSetId, synonymRule); + return Objects.hash(synonymsSetId, synonymRule, timeout); } } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java index 0d55774489c4c..0a7fc5f8e1a39 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java @@ -41,7 +41,7 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act synonymsManagementAPIService.deleteSynonymRule( request.synonymsSetId(), request.synonymRuleId(), - listener.map(SynonymUpdateResponse::new) - ); + listener.map(SynonymUpdateResponse::new), + timeout); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java index a6dc43cb55064..d53f2b1258a98 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java @@ -39,6 +39,6 @@ public TransportDeleteSynonymsAction(TransportService transportService, ActionFi @Override protected void doExecute(Task task, DeleteSynonymsAction.Request request, ActionListener listener) { - synonymsManagementAPIService.deleteSynonymsSet(request.synonymsSetId(), listener); + synonymsManagementAPIService.deleteSynonymsSet(request.synonymsSetId(), listener, timeout); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java index 33baa7b9081f2..3173ec5af54bd 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java @@ -41,8 +41,7 @@ protected void doExecute(Task task, PutSynonymRuleAction.Request request, Action synonymsManagementAPIService.putSynonymRule( request.synonymsSetId(), request.synonymRule(), - listener.map(updateResponse -> new SynonymUpdateResponse(updateResponse)) - ); - + listener.map(SynonymUpdateResponse::new), + request.timeout()); } } diff --git a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java index 44b8ff4dc2a8f..a0e270ca670a4 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java +++ b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java @@ -158,6 +158,23 @@ public void clusterChanged(ClusterChangedEvent event) { } for (IndexRoutingTable indexRoutingTable : routingTable) { + indexRoutingTable.allShards().forEach(shardRouting -> { + shardRouting.allShards().forEach(sr -> { + System.out.println( + "Shard " + + sr.currentNodeId() + + "/" + + sr.index() + + "/" + + sr.id() + + " - State " + + sr.state() + + " " + + sr.shortSummary() + ); + }); + }); + // Note, closed indices will not have any routing information, so won't be deleted for (int i = 0; i < indexRoutingTable.size(); i++) { IndexShardRoutingTable indexShardRoutingTable = indexRoutingTable.shard(i); diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java index db912f9d81d76..bb4e28455c7e2 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java @@ -41,6 +41,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient PutSynonymRuleAction.Request request = new PutSynonymRuleAction.Request( restRequest.param("synonymsSet"), restRequest.param("synonymRuleId"), + restRequest.paramAsInt("timeout", PutSynonymRuleAction.DEFAULT_TIMEOUT), restRequest.content(), restRequest.getXContentType() ); diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index a3d3af70de28a..cd7c662f1e1e8 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -19,6 +19,8 @@ import org.elasticsearch.action.DelegatingActionListener; import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; +import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest; +import org.elasticsearch.action.admin.cluster.health.TransportClusterHealthAction; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersRequest; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse; import org.elasticsearch.action.admin.indices.analyze.TransportReloadAnalyzersAction; @@ -36,12 +38,14 @@ import org.elasticsearch.cluster.metadata.IndexMetadata; import org.elasticsearch.cluster.routing.Preference; import org.elasticsearch.common.settings.Settings; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.IndexNotFoundException; import org.elasticsearch.index.query.BoolQueryBuilder; import org.elasticsearch.index.query.QueryBuilders; import org.elasticsearch.index.reindex.BulkByScrollResponse; import org.elasticsearch.index.reindex.DeleteByQueryAction; import org.elasticsearch.index.reindex.DeleteByQueryRequest; +import org.elasticsearch.indices.IndexCreationException; import org.elasticsearch.indices.SystemIndexDescriptor; import org.elasticsearch.rest.RestStatus; import org.elasticsearch.search.aggregations.BucketOrder; @@ -301,7 +305,7 @@ private static void logUniqueFailureMessagesWithIndices(List listener) { + public void putSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, int timeout, ActionListener listener) { if (synonymsSet.length > maxSynonymsSets) { listener.onFailure( new IllegalArgumentException("The number of synonyms rules in a synonym set cannot exceed " + maxSynonymsSets) @@ -343,7 +347,13 @@ public void putSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - reloadAnalyzers(synonymSetId, false, bulkInsertResponseListener, updateSynonymsResultStatus); + ensureSynonymsSearchableAndReloadAnalyzers( + synonymSetId, + false, + bulkInsertResponseListener, + updateSynonymsResultStatus, + timeout + ); }) ); })); @@ -366,7 +376,7 @@ void bulkUpdateSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio bulkRequestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).execute(listener); } - public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener) { + public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener, int timeout) { checkSynonymSetExists(synonymsSetId, listener.delegateFailureAndWrap((l1, obj) -> { // Count synonym rules to check if we're at maximum BoolQueryBuilder queryFilter = QueryBuilders.boolQuery() @@ -388,13 +398,13 @@ public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, Action new IllegalArgumentException("The number of synonym rules in a synonyms set cannot exceed " + maxSynonymsSets) ); } else { - indexSynonymRule(synonymsSetId, synonymRule, searchListener); + indexSynonymRule(synonymsSetId, synonymRule, searchListener, timeout); } })); })); } - private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener) + private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener, int timeout) throws IOException { IndexRequest indexRequest = createSynonymRuleIndexRequest(synonymsSetId, synonymRule).setRefreshPolicy( WriteRequest.RefreshPolicy.IMMEDIATE @@ -404,7 +414,7 @@ private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, Act ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - reloadAnalyzers(synonymsSetId, false, l2, updateStatus); + ensureSynonymsSearchableAndReloadAnalyzers(synonymsSetId, false, l2, updateStatus, timeout); })); } @@ -424,7 +434,7 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList ); } - public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener) { + public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener, int timeout) { client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId)) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> { @@ -443,7 +453,7 @@ public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, Action return; } - reloadAnalyzers(synonymsSetId, false, listener, UpdateSynonymsResultStatus.DELETED); + ensureSynonymsSearchableAndReloadAnalyzers(synonymsSetId, false, listener, UpdateSynonymsResultStatus.DELETED, timeout); })); } @@ -498,10 +508,10 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener) { + public void deleteSynonymsSet(String synonymSetId, ActionListener listener, int timeout) { // Previews reloading the resource to understand its usage on indices - reloadAnalyzers(synonymSetId, true, listener.delegateFailure((reloadListener, reloadResult) -> { + ensureSynonymsSearchableAndReloadAnalyzers(synonymSetId, true, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -538,16 +548,41 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void reloadAnalyzers( + private void ensureSynonymsSearchableAndReloadAnalyzers( String synonymSetId, boolean preview, ActionListener listener, - UpdateSynonymsResultStatus synonymsOperationResult + UpdateSynonymsResultStatus synonymsOperationResult, + int timeout ) { - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + // Ensure synonyms index is searchable if timeout is present + if (timeout > 0) { + ClusterHealthRequest healthRequest = new ClusterHealthRequest(TimeValue.timeValueSeconds(timeout), SYNONYMS_ALIAS_NAME) + .waitForGreenStatus(); + client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener.delegateFailure((l, response) -> { + if (response.isTimedOut()) { + l.onFailure(new IndexCreationException("synonyms index [" + SYNONYMS_ALIAS_NAME + "] is not searchable. " + + response.getActiveShardsPercent() + "% shards are active", null)); + return; + } + + reloadAnalyzers(synonymSetId, preview, l, synonymsOperationResult); + })); + } else { + reloadAnalyzers(synonymSetId, preview, listener, synonymsOperationResult); + } + } + + private void reloadAnalyzers( + String synonymSetId, + boolean preview, + ActionListener listener, + UpdateSynonymsResultStatus synonymsOperationResult) { + + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); client.execute( TransportReloadAnalyzersAction.TYPE, From a5a8a75a29ee11ea092d76d0b4faf87d101576dd Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 11:34:03 +0200 Subject: [PATCH 02/36] Remove replicas 0, as that may impact serverless --- .../rest-api-spec/test/synonyms/110_synonyms_invalid.yml | 7 ------- .../rest-api-spec/test/synonyms/30_synonyms_delete.yml | 1 - .../rest-api-spec/test/synonyms/80_synonyms_from_index.yml | 1 - 3 files changed, 9 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml index f4578290788f1..658e17b641118 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml @@ -24,7 +24,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -79,7 +78,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -186,7 +184,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -232,7 +229,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -326,7 +322,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -378,7 +373,6 @@ setup: index: .synonyms wait_for_status: green - - do: indices.stats: { index: test_index } @@ -406,7 +400,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml index ea27267c518a5..4fc34d1c30f9a 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml @@ -77,7 +77,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml index a996357896594..d813e175de335 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml @@ -180,7 +180,6 @@ setup: settings: index: number_of_shards: 1 - number_of_replicas: 0 analysis: filter: my_synonym_filter: From 6e57ae3fe0fa999e6c6f0d3468887bd1cd6a795b Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 11:49:30 +0200 Subject: [PATCH 03/36] Add timeout to put synonyms action, fix tests --- .../action/synonyms/PutSynonymsAction.java | 28 ++++++++++++++++--- .../TransportDeleteSynonymRuleAction.java | 4 +-- .../TransportDeleteSynonymsAction.java | 2 +- .../synonyms/TransportPutSynonymsAction.java | 1 + .../synonyms/RestPutSynonymsAction.java | 2 ++ .../SynonymsManagementAPIService.java | 10 +++---- ...onymRuleActionRequestSerializingTests.java | 2 +- ...SynonymsActionRequestSerializingTests.java | 2 +- 8 files changed, 37 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java index 5e806ef2ef548..adaafa836ae59 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; @@ -31,6 +32,8 @@ import java.util.List; import java.util.Objects; +import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; + public class PutSynonymsAction extends ActionType { public static final PutSynonymsAction INSTANCE = new PutSynonymsAction(); @@ -43,6 +46,7 @@ public PutSynonymsAction() { public static class Request extends ActionRequest { private final String synonymsSetId; private final SynonymRule[] synonymRules; + private final int timeout; public static final ParseField SYNONYMS_SET_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_SET_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("synonyms_set", args -> { @@ -59,10 +63,16 @@ public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRules = in.readArray(SynonymRule::new, SynonymRule[]::new); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { + this.timeout = in.readInt(); + } else { + this.timeout = DEFAULT_TIMEOUT; + } } - public Request(String synonymsSetId, BytesReference content, XContentType contentType) throws IOException { + public Request(String synonymsSetId, int timeout, BytesReference content, XContentType contentType) throws IOException { this.synonymsSetId = synonymsSetId; + this.timeout = timeout; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { this.synonymRules = PARSER.apply(parser, null); } catch (Exception e) { @@ -70,9 +80,10 @@ public Request(String synonymsSetId, BytesReference content, XContentType conten } } - Request(String synonymsSetId, SynonymRule[] synonymRules) { + Request(String synonymsSetId, SynonymRule[] synonymRules, int timeout) { this.synonymsSetId = synonymsSetId; this.synonymRules = synonymRules; + this.timeout = timeout; } @Override @@ -95,12 +106,19 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); out.writeArray(synonymRules); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { + out.writeInt(timeout); + } } public String synonymsSetId() { return synonymsSetId; } + public int timeout() { + return timeout; + } + public SynonymRule[] synonymRules() { return synonymRules; } @@ -110,12 +128,14 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Request request = (Request) o; - return Objects.equals(synonymsSetId, request.synonymsSetId) && Arrays.equals(synonymRules, request.synonymRules); + return timeout == request.timeout + && Objects.equals(synonymsSetId, request.synonymsSetId) + && Arrays.equals(synonymRules, request.synonymRules); } @Override public int hashCode() { - return Objects.hash(synonymsSetId, Arrays.hashCode(synonymRules)); + return Objects.hash(synonymsSetId, Arrays.hashCode(synonymRules), timeout); } } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java index 0a7fc5f8e1a39..0d55774489c4c 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java @@ -41,7 +41,7 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act synonymsManagementAPIService.deleteSynonymRule( request.synonymsSetId(), request.synonymRuleId(), - listener.map(SynonymUpdateResponse::new), - timeout); + listener.map(SynonymUpdateResponse::new) + ); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java index d53f2b1258a98..a6dc43cb55064 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java @@ -39,6 +39,6 @@ public TransportDeleteSynonymsAction(TransportService transportService, ActionFi @Override protected void doExecute(Task task, DeleteSynonymsAction.Request request, ActionListener listener) { - synonymsManagementAPIService.deleteSynonymsSet(request.synonymsSetId(), listener, timeout); + synonymsManagementAPIService.deleteSynonymsSet(request.synonymsSetId(), listener); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java index b0095ebee6a05..8a738b0ba65b8 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java @@ -35,6 +35,7 @@ protected void doExecute(Task task, PutSynonymsAction.Request request, ActionLis synonymsManagementAPIService.putSynonymsSet( request.synonymsSetId(), request.synonymRules(), + request.timeout(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index 3244f7960bdcc..d5b1ca20f12bb 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) @@ -40,6 +41,7 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { PutSynonymsAction.Request request = new PutSynonymsAction.Request( restRequest.param("synonymsSet"), + restRequest.paramAsInt("timeout", DEFAULT_TIMEOUT), restRequest.content(), restRequest.getXContentType() ); diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index cd7c662f1e1e8..a8768638875a0 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -434,7 +434,7 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList ); } - public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener, int timeout) { + public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener) { client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId)) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> { @@ -453,7 +453,7 @@ public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, Action return; } - ensureSynonymsSearchableAndReloadAnalyzers(synonymsSetId, false, listener, UpdateSynonymsResultStatus.DELETED, timeout); + reloadAnalyzers(synonymsSetId, false, listener, UpdateSynonymsResultStatus.DELETED); })); } @@ -508,10 +508,10 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener, int timeout) { + public void deleteSynonymsSet(String synonymSetId, ActionListener listener) { // Previews reloading the resource to understand its usage on indices - ensureSynonymsSearchableAndReloadAnalyzers(synonymSetId, true, listener.delegateFailure((reloadListener, reloadResult) -> { + reloadAnalyzers(synonymSetId, true, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -548,7 +548,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void ensureSynonymsSearchableAndReloadAnalyzers( diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java index 0f7a2b08fcdae..3fdbb32bc7646 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java @@ -22,7 +22,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymRuleAction.Request createTestInstance() { - return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule()); + return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule(), randomNonNegativeInt()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java index d8e73c517f84b..d252ecb87190a 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java @@ -25,7 +25,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymsAction.Request createTestInstance() { - return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet()); + return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet(), randomNonNegativeInt()); } @Override From 7dee4b5d5ea7f0aee033d71c7fced5de0e74d74c Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 12:01:42 +0200 Subject: [PATCH 04/36] Fix number of replicas --- .../rest-api-spec/test/synonyms/110_synonyms_invalid.yml | 6 ++++++ .../rest-api-spec/test/synonyms/80_synonyms_from_index.yml | 1 + 2 files changed, 7 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml index 658e17b641118..b2fbd185b8cf5 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml @@ -24,6 +24,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -78,6 +79,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -184,6 +186,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -229,6 +232,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -322,6 +326,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: @@ -400,6 +405,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml index d813e175de335..a996357896594 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml @@ -180,6 +180,7 @@ setup: settings: index: number_of_shards: 1 + number_of_replicas: 0 analysis: filter: my_synonym_filter: From 19b5c338f7d51b638fac4bb033d87cd438182666 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 12:03:41 +0200 Subject: [PATCH 05/36] Remove cluster.health checks for synonyms index --- .../test/synonyms/10_synonyms_put.yml | 10 ---------- .../test/synonyms/110_synonyms_invalid.yml | 18 ------------------ .../test/synonyms/20_synonyms_get.yml | 6 ------ .../test/synonyms/30_synonyms_delete.yml | 6 ------ .../test/synonyms/40_synonyms_sets_get.yml | 6 ------ .../test/synonyms/50_synonym_rule_put.yml | 6 ------ .../test/synonyms/60_synonym_rule_get.yml | 6 ------ .../test/synonyms/70_synonym_rule_delete.yml | 6 ------ .../test/synonyms/80_synonyms_from_index.yml | 6 ------ .../90_synonyms_reloading_for_synset.yml | 6 ------ 10 files changed, 76 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml index 4f36514f833dc..bcd58f3f7bd64 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml @@ -15,11 +15,6 @@ setup: - match: { result: "created" } - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: synonyms.get_synonym: id: test-update-synonyms @@ -63,11 +58,6 @@ setup: - match: { result: "created" } - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: synonyms.get_synonym: id: test-empty-synonyms diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml index b2fbd185b8cf5..131c7c0e826fc 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/110_synonyms_invalid.yml @@ -11,12 +11,6 @@ setup: synonyms_set: synonyms: "foo => bar, baz" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: indices.create: index: test_index @@ -372,12 +366,6 @@ setup: synonyms_set: synonyms: "foo => bar, baz" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: indices.stats: { index: test_index } @@ -440,12 +428,6 @@ setup: synonyms_set: synonyms: "foo => bar, baz" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: # Warning issued in previous versions allowed_warnings: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml index 8ab97b3ec779d..20f76e31e43a8 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/20_synonyms_get.yml @@ -14,12 +14,6 @@ setup: - synonyms: "test => check" id: "test-id-3" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - --- "Get synonyms set": - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml index 4fc34d1c30f9a..51d2a9c5a6938 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/30_synonyms_delete.yml @@ -12,12 +12,6 @@ setup: - synonyms: "bye => goodbye" id: "test-id-2" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - --- "Delete synonyms set": - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml index e68c93077bdec..9d6540c118ce5 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/40_synonyms_sets_get.yml @@ -10,12 +10,6 @@ setup: - synonyms: "hello, hi" - synonyms: "goodbye, bye" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - - do: synonyms.put_synonym: id: test-synonyms-1 diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml index c8f463ba5cbe7..72e4d17738375 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml @@ -14,12 +14,6 @@ setup: - synonyms: "test => check" id: "test-id-3" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - --- "Update a synonyms rule": - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml index 1754467e89b2f..413337072e160 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/60_synonym_rule_get.yml @@ -14,12 +14,6 @@ setup: - synonyms: "test => check" id: "test-id-3" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - --- "Get a synonym rule": - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml index b24ed799bfd8f..a4853b0b6d414 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml @@ -14,12 +14,6 @@ setup: - synonyms: "test => check" id: "test-id-3" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - --- "Delete synonym rule": - do: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml index a996357896594..15cb1000e2ad3 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/80_synonyms_from_index.yml @@ -13,12 +13,6 @@ setup: - synonyms: "bye => goodbye" id: "synonym-rule-2" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - # Create an index with synonym_filter that uses that synonyms set - do: indices.create: diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml index 02db799e52e51..579d99e3f2ff7 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml @@ -14,12 +14,6 @@ setup: - synonyms: "bye => goodbye" id: "synonym-rule-2" - # This is to ensure that all index shards (write and read) are available. In serverless this can take some time. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - # Create synonyms synonyms_set2 - do: synonyms.put_synonym: From fcf4dc6f74b4eaf9855e17bcd514798119659ab3 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 12:06:01 +0200 Subject: [PATCH 06/36] Revert debugging --- .../indices/store/IndicesStore.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java index a0e270ca670a4..44b8ff4dc2a8f 100644 --- a/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java +++ b/server/src/main/java/org/elasticsearch/indices/store/IndicesStore.java @@ -158,23 +158,6 @@ public void clusterChanged(ClusterChangedEvent event) { } for (IndexRoutingTable indexRoutingTable : routingTable) { - indexRoutingTable.allShards().forEach(shardRouting -> { - shardRouting.allShards().forEach(sr -> { - System.out.println( - "Shard " - + sr.currentNodeId() - + "/" - + sr.index() - + "/" - + sr.id() - + " - State " - + sr.state() - + " " - + sr.shortSummary() - ); - }); - }); - // Note, closed indices will not have any routing information, so won't be deleted for (int i = 0; i < indexRoutingTable.size(); i++) { IndexShardRoutingTable indexShardRoutingTable = indexRoutingTable.shard(i); From 51f67eb42a46bd25e3257eaa9aacd8ca1453703d Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 12:58:16 +0200 Subject: [PATCH 07/36] Add integration test for timeouts --- .../SynonymsManagementAPIServiceIT.java | 128 ++++++++++++++++-- .../TransportPutSynonymRuleAction.java | 4 +- .../SynonymsManagementAPIService.java | 19 ++- 3 files changed, 133 insertions(+), 18 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index fea567857ea89..18d943c942254 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -11,8 +11,12 @@ import org.apache.logging.log4j.Logger; import org.elasticsearch.action.ActionListener; +import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.bulk.BulkResponse; +import org.elasticsearch.cluster.ClusterName; +import org.elasticsearch.cluster.ClusterState; import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; +import org.elasticsearch.indices.IndexCreationException; import org.elasticsearch.plugins.Plugin; import org.elasticsearch.reindex.ReindexPlugin; import org.elasticsearch.test.ESIntegTestCase; @@ -137,7 +141,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); for (int i = 0; i < rulesToUpdate; i++) { - synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], new ActionListener<>() { + synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { updatedRulesLatch.countDown(); @@ -147,7 +151,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { fail(e); } - }, DEFAULT_TIMEOUT); + }); } try { updatedRulesLatch.await(5, TimeUnit.SECONDS); @@ -163,7 +167,7 @@ public void onFailure(Exception e) { // Error here synonymSetId, rules[i], - new ActionListener<>() { + DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { fail("Shouldn't have been able to update a rule"); @@ -176,8 +180,8 @@ public void onFailure(Exception e) { } updatedRulesLatch.countDown(); } - }, - DEFAULT_TIMEOUT); + } + ); } try { insertRulesLatch.await(5, TimeUnit.SECONDS); @@ -206,7 +210,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym synonymsManagementAPIService.putSynonymRule( synonymSetId, synonymsSet[randomIntBetween(0, maxSynonymSets - 1)], - new ActionListener<>() { + DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { latch.countDown(); @@ -216,8 +220,8 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { fail("Should update a rule that already exists at max capcity"); } - }, - DEFAULT_TIMEOUT); + } + ); } @Override @@ -238,7 +242,7 @@ public void testCreateRuleWithMaxSynonyms() throws InterruptedException { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails - synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(ruleId), new ActionListener<>() { + synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(ruleId), DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { fail("Should not create a new rule that does not exist when at max capacity"); @@ -248,7 +252,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { latch.countDown(); } - }, DEFAULT_TIMEOUT); + }); } @Override @@ -299,4 +303,108 @@ public void onFailure(Exception e) { readLatch.await(5, TimeUnit.SECONDS); verify(logger).warn(anyString(), eq(synonymSetId)); } + + public void testCreateSynonymsWithYellowSynonymsIndex() throws Exception { + + // Override health method check to simulate a timeout in checking the synonyms index + synonymsManagementAPIService = new SynonymsManagementAPIService(client()) { + @Override + void checkSynonymsIndexHealth(int timeout, ActionListener listener) { + ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); + ClusterHealthResponse response = new ClusterHealthResponse(randomIdentifier(), + new String[]{SynonymsManagementAPIService.SYNONYMS_INDEX_CONCRETE_NAME}, + clusterState); + response.setTimedOut(true); + listener.onResponse(response); + } + }; + + // Create a rule fails + CountDownLatch putLatch = new CountDownLatch(1); + String synonymSetId = randomIdentifier(); + synonymsManagementAPIService.putSynonymsSet( + synonymSetId, + randomSynonymsSet(1, 1), + DEFAULT_TIMEOUT, + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to create synonyms with a timeout in synonyms index health"); + } + + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + putLatch.countDown(); + } + }); + + putLatch.await(5, TimeUnit.SECONDS); + + // Update a rule fails + CountDownLatch updateLatch = new CountDownLatch(1); + synonymsManagementAPIService.putSynonymRule( + synonymSetId, + randomSynonymRule(randomIdentifier()), + DEFAULT_TIMEOUT, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to update synonyms with a timeout in synonyms index health"); + } + + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + updateLatch.countDown(); + } + }); + + updateLatch.await(5, TimeUnit.SECONDS); + + // But, we can still create a synonyms set with timeout 0 + CountDownLatch putWithoutTimeoutLatch = new CountDownLatch(1); + synonymsManagementAPIService.putSynonymsSet( + synonymSetId, + randomSynonymsSet(1, 1), + 0, + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Expected + putLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); + + putWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); + + // Same for update + CountDownLatch putRuleWithoutTimeoutLatch = new CountDownLatch(1); + synonymsManagementAPIService.putSynonymRule( + synonymSetId, + randomSynonymRule(randomIdentifier()), + 0, + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Expected + putRuleWithoutTimeoutLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); + + putRuleWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); + } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java index 3173ec5af54bd..0c8705bb6d177 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java @@ -41,7 +41,7 @@ protected void doExecute(Task task, PutSynonymRuleAction.Request request, Action synonymsManagementAPIService.putSynonymRule( request.synonymsSetId(), request.synonymRule(), - listener.map(SynonymUpdateResponse::new), - request.timeout()); + request.timeout(), listener.map(SynonymUpdateResponse::new) + ); } } diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index a8768638875a0..a0f8049265382 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -20,6 +20,7 @@ import org.elasticsearch.action.DocWriteRequest; import org.elasticsearch.action.DocWriteResponse; import org.elasticsearch.action.admin.cluster.health.ClusterHealthRequest; +import org.elasticsearch.action.admin.cluster.health.ClusterHealthResponse; import org.elasticsearch.action.admin.cluster.health.TransportClusterHealthAction; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersRequest; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse; @@ -76,7 +77,7 @@ public class SynonymsManagementAPIService { private static final String SYNONYMS_INDEX_NAME_PATTERN = ".synonyms-*"; private static final int SYNONYMS_INDEX_FORMAT = 2; - private static final String SYNONYMS_INDEX_CONCRETE_NAME = ".synonyms-" + SYNONYMS_INDEX_FORMAT; + static final String SYNONYMS_INDEX_CONCRETE_NAME = ".synonyms-" + SYNONYMS_INDEX_FORMAT; private static final String SYNONYMS_ALIAS_NAME = ".synonyms"; public static final String SYNONYMS_FEATURE_NAME = "synonyms"; // Stores the synonym set the rule belongs to @@ -376,7 +377,7 @@ void bulkUpdateSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio bulkRequestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).execute(listener); } - public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener, int timeout) { + public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, int timeout, ActionListener listener) { checkSynonymSetExists(synonymsSetId, listener.delegateFailureAndWrap((l1, obj) -> { // Count synonym rules to check if we're at maximum BoolQueryBuilder queryFilter = QueryBuilders.boolQuery() @@ -560,9 +561,7 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( ) { // Ensure synonyms index is searchable if timeout is present if (timeout > 0) { - ClusterHealthRequest healthRequest = new ClusterHealthRequest(TimeValue.timeValueSeconds(timeout), SYNONYMS_ALIAS_NAME) - .waitForGreenStatus(); - client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener.delegateFailure((l, response) -> { + checkSynonymsIndexHealth(timeout, listener.delegateFailure((l, response) -> { if (response.isTimedOut()) { l.onFailure(new IndexCreationException("synonyms index [" + SYNONYMS_ALIAS_NAME + "] is not searchable. " + response.getActiveShardsPercent() + "% shards are active", null)); @@ -576,6 +575,14 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( } } + // Allows checking failures in tests + void checkSynonymsIndexHealth(int timeout, ActionListener listener) { + ClusterHealthRequest healthRequest = new ClusterHealthRequest(TimeValue.timeValueSeconds(timeout), SYNONYMS_ALIAS_NAME) + .waitForGreenStatus(); + + client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener); + } + private void reloadAnalyzers( String synonymSetId, boolean preview, @@ -597,7 +604,7 @@ private static String internalSynonymRuleId(String synonymsSetId, String synonym return synonymsSetId + SYNONYM_RULE_ID_SEPARATOR + synonymRuleId; } - static Settings settings() { + private static Settings settings() { return Settings.builder() .put(IndexMetadata.SETTING_NUMBER_OF_SHARDS, 1) .put(IndexMetadata.SETTING_AUTO_EXPAND_REPLICAS, "0-1") From ab0805d60d8ee60a81fda2ab10fed6259f82510d Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 13:22:41 +0200 Subject: [PATCH 08/36] Use TimeValue instead of an int --- .../SynonymsManagementAPIServiceIT.java | 9 +++---- .../action/synonyms/PutSynonymRuleAction.java | 24 ++++++++----------- .../action/synonyms/PutSynonymsAction.java | 18 +++++++++----- .../synonyms/RestPutSynonymRuleAction.java | 3 ++- .../synonyms/RestPutSynonymsAction.java | 3 ++- .../SynonymsManagementAPIService.java | 16 ++++++------- ...onymRuleActionRequestSerializingTests.java | 2 +- ...SynonymsActionRequestSerializingTests.java | 2 +- 8 files changed, 41 insertions(+), 36 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 18d943c942254..0a60418945b50 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -15,6 +15,7 @@ import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; import org.elasticsearch.indices.IndexCreationException; import org.elasticsearch.plugins.Plugin; @@ -309,7 +310,7 @@ public void testCreateSynonymsWithYellowSynonymsIndex() throws Exception { // Override health method check to simulate a timeout in checking the synonyms index synonymsManagementAPIService = new SynonymsManagementAPIService(client()) { @Override - void checkSynonymsIndexHealth(int timeout, ActionListener listener) { + void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); ClusterHealthResponse response = new ClusterHealthResponse(randomIdentifier(), new String[]{SynonymsManagementAPIService.SYNONYMS_INDEX_CONCRETE_NAME}, @@ -365,12 +366,12 @@ public void onFailure(Exception e) { updateLatch.await(5, TimeUnit.SECONDS); - // But, we can still create a synonyms set with timeout 0 + // But, we can still create a synonyms set with timeout -1 CountDownLatch putWithoutTimeoutLatch = new CountDownLatch(1); synonymsManagementAPIService.putSynonymsSet( synonymSetId, randomSynonymsSet(1, 1), - 0, + TimeValue.MINUS_ONE, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { @@ -391,7 +392,7 @@ public void onFailure(Exception e) { synonymsManagementAPIService.putSynonymRule( synonymSetId, randomSynonymRule(randomIdentifier()), - 0, + TimeValue.MINUS_ONE, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java index 2c4abb39d1fb0..d812e581eed10 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.synonyms.SynonymRule; import org.elasticsearch.synonyms.SynonymsManagementAPIService; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -35,7 +36,7 @@ public class PutSynonymRuleAction extends ActionType { public static final PutSynonymRuleAction INSTANCE = new PutSynonymRuleAction(); public static final String NAME = "cluster:admin/synonym_rules/put"; - public static final int DEFAULT_TIMEOUT = 5; + public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(10); public PutSynonymRuleAction() { super(NAME); @@ -44,7 +45,7 @@ public PutSynonymRuleAction() { public static class Request extends ActionRequest { private final String synonymsSetId; private final SynonymRule synonymRule; - private final int timeout; + private final TimeValue timeout; public static final ParseField SYNONYMS_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( @@ -62,14 +63,15 @@ public Request(StreamInput in) throws IOException { this.synonymsSetId = in.readString(); this.synonymRule = new SynonymRule(in); if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - this.timeout = in.readInt(); + this.timeout = in.readTimeValue(); } else { this.timeout = DEFAULT_TIMEOUT; } } - public Request(String synonymsSetId, String synonymRuleId, int timeout, BytesReference content, XContentType contentType) + public Request(String synonymsSetId, String synonymRuleId, TimeValue timeout, BytesReference content, XContentType contentType) throws IOException { + Objects.requireNonNull(timeout); this.synonymsSetId = synonymsSetId; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { this.synonymRule = PARSER.apply(parser, synonymRuleId); @@ -79,7 +81,8 @@ public Request(String synonymsSetId, String synonymRuleId, int timeout, BytesRef this.timeout = timeout; } - Request(String synonymsSetId, SynonymRule synonymRule, int timeout) { + Request(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout) { + Objects.requireNonNull(timeout); this.synonymsSetId = synonymsSetId; this.synonymRule = synonymRule; this.timeout = timeout; @@ -94,13 +97,6 @@ public ActionRequestValidationException validate() { if (Strings.isNullOrEmpty(synonymRule.id())) { validationException = ValidateActions.addValidationError("synonym rule id must be specified", validationException); } - if (timeout < 0) { - validationException = ValidateActions.addValidationError("timeout must be greater than or equal to 0", validationException); - } - String error = synonymRule.validate(); - if (error != null) { - validationException = ValidateActions.addValidationError(error, validationException); - } return validationException; } @@ -111,7 +107,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(synonymsSetId); synonymRule.writeTo(out); if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - out.writeInt(timeout); + out.writeTimeValue(timeout); } } @@ -123,7 +119,7 @@ public SynonymRule synonymRule() { return synonymRule; } - public int timeout() { + public TimeValue timeout() { return timeout; } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java index adaafa836ae59..91b1b3522cdb9 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java @@ -19,6 +19,7 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.core.TimeValue; import org.elasticsearch.synonyms.SynonymRule; import org.elasticsearch.synonyms.SynonymsManagementAPIService; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -46,7 +47,7 @@ public PutSynonymsAction() { public static class Request extends ActionRequest { private final String synonymsSetId; private final SynonymRule[] synonymRules; - private final int timeout; + private final TimeValue timeout; public static final ParseField SYNONYMS_SET_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_SET_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("synonyms_set", args -> { @@ -64,13 +65,15 @@ public Request(StreamInput in) throws IOException { this.synonymsSetId = in.readString(); this.synonymRules = in.readArray(SynonymRule::new, SynonymRule[]::new); if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - this.timeout = in.readInt(); + this.timeout = in.readTimeValue(); } else { this.timeout = DEFAULT_TIMEOUT; } } - public Request(String synonymsSetId, int timeout, BytesReference content, XContentType contentType) throws IOException { + public Request(String synonymsSetId, TimeValue timeout, BytesReference content, XContentType contentType) throws IOException { + Objects.requireNonNull(timeout); + Objects.requireNonNull(synonymsSetId); this.synonymsSetId = synonymsSetId; this.timeout = timeout; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { @@ -80,7 +83,10 @@ public Request(String synonymsSetId, int timeout, BytesReference content, XConte } } - Request(String synonymsSetId, SynonymRule[] synonymRules, int timeout) { + Request(String synonymsSetId, SynonymRule[] synonymRules, TimeValue timeout) { + Objects.requireNonNull(timeout); + Objects.requireNonNull(synonymsSetId); + Objects.requireNonNull(synonymRules); this.synonymsSetId = synonymsSetId; this.synonymRules = synonymRules; this.timeout = timeout; @@ -107,7 +113,7 @@ public void writeTo(StreamOutput out) throws IOException { out.writeString(synonymsSetId); out.writeArray(synonymRules); if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - out.writeInt(timeout); + out.writeTimeValue(timeout); } } @@ -115,7 +121,7 @@ public String synonymsSetId() { return synonymsSetId; } - public int timeout() { + public TimeValue timeout() { return timeout; } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java index bb4e28455c7e2..b749366303154 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java @@ -14,6 +14,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; @@ -41,7 +42,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient PutSynonymRuleAction.Request request = new PutSynonymRuleAction.Request( restRequest.param("synonymsSet"), restRequest.param("synonymRuleId"), - restRequest.paramAsInt("timeout", PutSynonymRuleAction.DEFAULT_TIMEOUT), + RestUtils.getAckTimeout(restRequest), restRequest.content(), restRequest.getXContentType() ); diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index d5b1ca20f12bb..fe701de6168ee 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -14,6 +14,7 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; +import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; @@ -41,7 +42,7 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { PutSynonymsAction.Request request = new PutSynonymsAction.Request( restRequest.param("synonymsSet"), - restRequest.paramAsInt("timeout", DEFAULT_TIMEOUT), + RestUtils.getAckTimeout(restRequest), restRequest.content(), restRequest.getXContentType() ); diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index a0f8049265382..08782577d477c 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -306,7 +306,7 @@ private static void logUniqueFailureMessagesWithIndices(List listener) { + public void putSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, TimeValue timeout, ActionListener listener) { if (synonymsSet.length > maxSynonymsSets) { listener.onFailure( new IllegalArgumentException("The number of synonyms rules in a synonym set cannot exceed " + maxSynonymsSets) @@ -377,7 +377,7 @@ void bulkUpdateSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio bulkRequestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).execute(listener); } - public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, int timeout, ActionListener listener) { + public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout, ActionListener listener) { checkSynonymSetExists(synonymsSetId, listener.delegateFailureAndWrap((l1, obj) -> { // Count synonym rules to check if we're at maximum BoolQueryBuilder queryFilter = QueryBuilders.boolQuery() @@ -399,13 +399,13 @@ public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, int ti new IllegalArgumentException("The number of synonym rules in a synonyms set cannot exceed " + maxSynonymsSets) ); } else { - indexSynonymRule(synonymsSetId, synonymRule, searchListener, timeout); + indexSynonymRule(synonymsSetId, synonymRule, timeout, searchListener); } })); })); } - private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, ActionListener listener, int timeout) + private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout, ActionListener listener) throws IOException { IndexRequest indexRequest = createSynonymRuleIndexRequest(synonymsSetId, synonymRule).setRefreshPolicy( WriteRequest.RefreshPolicy.IMMEDIATE @@ -557,10 +557,10 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( boolean preview, ActionListener listener, UpdateSynonymsResultStatus synonymsOperationResult, - int timeout + TimeValue timeout ) { // Ensure synonyms index is searchable if timeout is present - if (timeout > 0) { + if (TimeValue.MINUS_ONE.equals(timeout) == false) { checkSynonymsIndexHealth(timeout, listener.delegateFailure((l, response) -> { if (response.isTimedOut()) { l.onFailure(new IndexCreationException("synonyms index [" + SYNONYMS_ALIAS_NAME + "] is not searchable. " @@ -576,8 +576,8 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( } // Allows checking failures in tests - void checkSynonymsIndexHealth(int timeout, ActionListener listener) { - ClusterHealthRequest healthRequest = new ClusterHealthRequest(TimeValue.timeValueSeconds(timeout), SYNONYMS_ALIAS_NAME) + void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { + ClusterHealthRequest healthRequest = new ClusterHealthRequest(timeout, SYNONYMS_ALIAS_NAME) .waitForGreenStatus(); client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener); diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java index 3fdbb32bc7646..db7c056557168 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java @@ -22,7 +22,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymRuleAction.Request createTestInstance() { - return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule(), randomNonNegativeInt()); + return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule(), randomTimeValue()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java index d252ecb87190a..9c67f32596c47 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java @@ -25,7 +25,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymsAction.Request createTestInstance() { - return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet(), randomNonNegativeInt()); + return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet(), randomTimeValue()); } @Override From 3b8bbfdd2894b3ad4b51241dee294e380b0255f8 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 16:43:51 +0200 Subject: [PATCH 09/36] Add YAML tests and REST API specs --- .../api/synonyms.put_synonym.json | 6 +++++ .../api/synonyms.put_synonym_rule.json | 6 +++++ .../test/synonyms/10_synonyms_put.yml | 26 +++++++++++++++++++ 3 files changed, 38 insertions(+) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json index e09bbb7e428a1..2ea7b34afc8a4 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json @@ -30,6 +30,12 @@ } ] }, + "params": { + "timeout": { + "type": "time", + "description": "Explicit timeout for the operation to complete" + } + }, "body": { "description": "Synonyms set rules", "required": true diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json index 51503b5819862..c4bd5ba46c50f 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json @@ -34,6 +34,12 @@ } ] }, + "params": { + "timeout": { + "type": "time", + "description": "Explicit timeout for the operation to complete" + } + }, "body": { "description": "Synonym rule", "required": true diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml index bcd58f3f7bd64..bcb9ac3e4db9c 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml @@ -65,6 +65,22 @@ setup: - match: { count: 0 } - match: { synonyms_set: [] } +--- +"Timeout can be specified": + - do: + synonyms.put_synonym: + id: test-update-synonyms + timeout: 10s + body: + synonyms_set: + - synonyms: "hello, hi" + - synonyms: "bye => goodbye" + id: "test-id" + + - match: { result: "created" } + - match: { reload_analyzers_details._shards.total: 0 } + - length: { reload_analyzers_details.reload_details: 0 } + --- "Validation fails tests": - do: @@ -106,3 +122,13 @@ setup: body: synonyms_set: - synonyms: "bye, goodbye, " + + - do: + catch: /as a time value:\ negative durations are not supported/ + synonyms.put_synonym: + id: test-update-synonyms + timeout: -100s + body: + synonyms_set: + - synonyms: "bye, goodbye" + From 8da76110ef95aa046b9673fceab290592e25ecb0 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 16:44:06 +0200 Subject: [PATCH 10/36] Fix a validation bug in put synonym rule --- .../test/synonyms/50_synonym_rule_put.yml | 66 +++++++++++++++++++ .../action/synonyms/PutSynonymRuleAction.java | 4 ++ 2 files changed, 70 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml index 72e4d17738375..abeb3078daed2 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml @@ -79,3 +79,69 @@ setup: rule_id: "test-id-0" body: synonyms: "i-phone, iphone" + +--- +"Timeout can be specified": + - do: + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + timeout: 10s + body: + synonyms: "i-phone, iphone" + + - match: { result: "created" } + - match: { reload_analyzers_details._shards.total: 0 } + - length: { reload_analyzers_details.reload_details: 0 } + +--- +"Validation failure tests": + - do: + catch: /\[synonyms\] field can't be empty/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + body: + synonyms: "" + + - do: + catch: /More than one explicit mapping specified in the same synonyms rule/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + body: + synonyms: "bye => => goodbye" + + - do: + catch: /Incorrect syntax for \[synonyms\]/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + body: + synonyms: " => goodbye" + + - do: + catch: /Incorrect syntax for \[synonyms\]/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + body: + synonyms: "bye => " + + - do: + catch: /Incorrect syntax for \[synonyms\]/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + body: + synonyms: "bye, goodbye, " + + - do: + catch: /as a time value:\ negative durations are not supported/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + timeout: -100s + body: + synonyms_set: + - synonyms: "bye, goodbye" diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java index d812e581eed10..a5e49bbad8a06 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java @@ -97,6 +97,10 @@ public ActionRequestValidationException validate() { if (Strings.isNullOrEmpty(synonymRule.id())) { validationException = ValidateActions.addValidationError("synonym rule id must be specified", validationException); } + String error = synonymRule.validate(); + if (error != null) { + validationException = ValidateActions.addValidationError(error, validationException); + } return validationException; } From 4ab01afcd1c1b9e8f97ded586aa4d703034ad8eb Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 16:45:29 +0200 Subject: [PATCH 11/36] Spotless --- .../SynonymsManagementAPIServiceIT.java | 218 +++++++++--------- .../TransportPutSynonymRuleAction.java | 3 +- .../synonyms/RestPutSynonymsAction.java | 1 - .../SynonymsManagementAPIService.java | 42 +++- 4 files changed, 146 insertions(+), 118 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 0a60418945b50..52b966922964e 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -63,19 +63,20 @@ public void testCreateManySynonyms() throws Exception { DEFAULT_TIMEOUT, new ActionListener<>() { @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - assertEquals( - SynonymsManagementAPIService.UpdateSynonymsResultStatus.CREATED, - synonymsReloadResult.synonymsOperationResult() - ); - putLatch.countDown(); - } + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + assertEquals( + SynonymsManagementAPIService.UpdateSynonymsResultStatus.CREATED, + synonymsReloadResult.synonymsOperationResult() + ); + putLatch.countDown(); + } - @Override - public void onFailure(Exception e) { - fail(e); + @Override + public void onFailure(Exception e) { + fail(e); + } } - }); + ); putLatch.await(5, TimeUnit.SECONDS); @@ -137,65 +138,67 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept DEFAULT_TIMEOUT, new ActionListener<>() { @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Create as many rules as should fail - SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); - CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); - for (int i = 0; i < rulesToUpdate; i++) { - synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], DEFAULT_TIMEOUT, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - updatedRulesLatch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); - } - try { - updatedRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - fail(e); - } - - // Updating more rules fails - int rulesToInsert = rules.length - rulesToUpdate; - CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); - for (int i = rulesToUpdate; i < rulesToInsert; i++) { - synonymsManagementAPIService.putSynonymRule( - // Error here - synonymSetId, - rules[i], - DEFAULT_TIMEOUT, new ActionListener<>() { + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Create as many rules as should fail + SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); + CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); + for (int i = 0; i < rulesToUpdate; i++) { + synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], DEFAULT_TIMEOUT, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update a rule"); + updatedRulesLatch.countDown(); } @Override public void onFailure(Exception e) { - if (e instanceof IllegalArgumentException == false) { - fail(e); + fail(e); + } + }); + } + try { + updatedRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } + + // Updating more rules fails + int rulesToInsert = rules.length - rulesToUpdate; + CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); + for (int i = rulesToUpdate; i < rulesToInsert; i++) { + synonymsManagementAPIService.putSynonymRule( + // Error here + synonymSetId, + rules[i], + DEFAULT_TIMEOUT, + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to update a rule"); + } + + @Override + public void onFailure(Exception e) { + if (e instanceof IllegalArgumentException == false) { + fail(e); + } + updatedRulesLatch.countDown(); } - updatedRulesLatch.countDown(); } - } - ); + ); + } + try { + insertRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } } - try { - insertRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { + + @Override + public void onFailure(Exception e) { fail(e); } } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); + ); latch.await(5, TimeUnit.SECONDS); } @@ -211,7 +214,8 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym synonymsManagementAPIService.putSynonymRule( synonymSetId, synonymsSet[randomIntBetween(0, maxSynonymSets - 1)], - DEFAULT_TIMEOUT, new ActionListener<>() { + DEFAULT_TIMEOUT, + new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { latch.countDown(); @@ -243,17 +247,22 @@ public void testCreateRuleWithMaxSynonyms() throws InterruptedException { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails - synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(ruleId), DEFAULT_TIMEOUT, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Should not create a new rule that does not exist when at max capacity"); - } + synonymsManagementAPIService.putSynonymRule( + synonymSetId, + randomSynonymRule(ruleId), + DEFAULT_TIMEOUT, + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Should not create a new rule that does not exist when at max capacity"); + } - @Override - public void onFailure(Exception e) { - latch.countDown(); + @Override + public void onFailure(Exception e) { + latch.countDown(); + } } - }); + ); } @Override @@ -312,9 +321,11 @@ public void testCreateSynonymsWithYellowSynonymsIndex() throws Exception { @Override void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); - ClusterHealthResponse response = new ClusterHealthResponse(randomIdentifier(), - new String[]{SynonymsManagementAPIService.SYNONYMS_INDEX_CONCRETE_NAME}, - clusterState); + ClusterHealthResponse response = new ClusterHealthResponse( + randomIdentifier(), + new String[] { SynonymsManagementAPIService.SYNONYMS_INDEX_CONCRETE_NAME }, + clusterState + ); response.setTimedOut(true); listener.onResponse(response); } @@ -323,24 +334,20 @@ void checkSynonymsIndexHealth(TimeValue timeout, ActionListener() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to create synonyms with a timeout in synonyms index health"); - } + synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), DEFAULT_TIMEOUT, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to create synonyms with a timeout in synonyms index health"); + } - @Override - public void onFailure(Exception e) { - // Expected - assertTrue(e instanceof IndexCreationException); - assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); - putLatch.countDown(); - } - }); + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + putLatch.countDown(); + } + }); putLatch.await(5, TimeUnit.SECONDS); @@ -349,7 +356,8 @@ public void onFailure(Exception e) { synonymsManagementAPIService.putSynonymRule( synonymSetId, randomSynonymRule(randomIdentifier()), - DEFAULT_TIMEOUT, new ActionListener<>() { + DEFAULT_TIMEOUT, + new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { fail("Shouldn't have been able to update synonyms with a timeout in synonyms index health"); @@ -362,28 +370,25 @@ public void onFailure(Exception e) { assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); updateLatch.countDown(); } - }); + } + ); updateLatch.await(5, TimeUnit.SECONDS); // But, we can still create a synonyms set with timeout -1 CountDownLatch putWithoutTimeoutLatch = new CountDownLatch(1); - synonymsManagementAPIService.putSynonymsSet( - synonymSetId, - randomSynonymsSet(1, 1), - TimeValue.MINUS_ONE, - new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Expected - putLatch.countDown(); - } + synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), TimeValue.MINUS_ONE, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Expected + putLatch.countDown(); + } - @Override - public void onFailure(Exception e) { - fail(e); - } - }); + @Override + public void onFailure(Exception e) { + fail(e); + } + }); putWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); @@ -404,7 +409,8 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym public void onFailure(Exception e) { fail(e); } - }); + } + ); putRuleWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java index 0c8705bb6d177..c9e24dfd78e8e 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java @@ -41,7 +41,8 @@ protected void doExecute(Task task, PutSynonymRuleAction.Request request, Action synonymsManagementAPIService.putSynonymRule( request.synonymsSetId(), request.synonymRule(), - request.timeout(), listener.map(SynonymUpdateResponse::new) + request.timeout(), + listener.map(SynonymUpdateResponse::new) ); } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index fe701de6168ee..a137234e9ed9c 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -22,7 +22,6 @@ import java.io.IOException; import java.util.List; -import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 08782577d477c..57658a014e72e 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -306,7 +306,12 @@ private static void logUniqueFailureMessagesWithIndices(List listener) { + public void putSynonymsSet( + String synonymSetId, + SynonymRule[] synonymsSet, + TimeValue timeout, + ActionListener listener + ) { if (synonymsSet.length > maxSynonymsSets) { listener.onFailure( new IllegalArgumentException("The number of synonyms rules in a synonym set cannot exceed " + maxSynonymsSets) @@ -377,7 +382,12 @@ void bulkUpdateSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio bulkRequestBuilder.setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE).execute(listener); } - public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout, ActionListener listener) { + public void putSynonymRule( + String synonymsSetId, + SynonymRule synonymRule, + TimeValue timeout, + ActionListener listener + ) { checkSynonymSetExists(synonymsSetId, listener.delegateFailureAndWrap((l1, obj) -> { // Count synonym rules to check if we're at maximum BoolQueryBuilder queryFilter = QueryBuilders.boolQuery() @@ -405,8 +415,12 @@ public void putSynonymRule(String synonymsSetId, SynonymRule synonymRule, TimeVa })); } - private void indexSynonymRule(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout, ActionListener listener) - throws IOException { + private void indexSynonymRule( + String synonymsSetId, + SynonymRule synonymRule, + TimeValue timeout, + ActionListener listener + ) throws IOException { IndexRequest indexRequest = createSynonymRuleIndexRequest(synonymsSetId, synonymRule).setRefreshPolicy( WriteRequest.RefreshPolicy.IMMEDIATE ); @@ -563,8 +577,16 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( if (TimeValue.MINUS_ONE.equals(timeout) == false) { checkSynonymsIndexHealth(timeout, listener.delegateFailure((l, response) -> { if (response.isTimedOut()) { - l.onFailure(new IndexCreationException("synonyms index [" + SYNONYMS_ALIAS_NAME + "] is not searchable. " - + response.getActiveShardsPercent() + "% shards are active", null)); + l.onFailure( + new IndexCreationException( + "synonyms index [" + + SYNONYMS_ALIAS_NAME + + "] is not searchable. " + + response.getActiveShardsPercent() + + "% shards are active", + null + ) + ); return; } @@ -577,8 +599,7 @@ private void ensureSynonymsSearchableAndReloadAnalyzers( // Allows checking failures in tests void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { - ClusterHealthRequest healthRequest = new ClusterHealthRequest(timeout, SYNONYMS_ALIAS_NAME) - .waitForGreenStatus(); + ClusterHealthRequest healthRequest = new ClusterHealthRequest(timeout, SYNONYMS_ALIAS_NAME).waitForGreenStatus(); client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener); } @@ -587,9 +608,10 @@ private void reloadAnalyzers( String synonymSetId, boolean preview, ActionListener listener, - UpdateSynonymsResultStatus synonymsOperationResult) { + UpdateSynonymsResultStatus synonymsOperationResult + ) { - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); client.execute( TransportReloadAnalyzersAction.TYPE, From 6245d57479453c871e76a93968b6abfe8fef6f79 Mon Sep 17 00:00:00 2001 From: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Date: Fri, 4 Apr 2025 16:52:31 +0200 Subject: [PATCH 12/36] Update docs/changelog/126314.yaml --- docs/changelog/126314.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/126314.yaml diff --git a/docs/changelog/126314.yaml b/docs/changelog/126314.yaml new file mode 100644 index 0000000000000..4336c3e39e10e --- /dev/null +++ b/docs/changelog/126314.yaml @@ -0,0 +1,6 @@ +pr: 126314 +summary: Add timeout to synonyms put APIs to wait for synonyms to be accessible +area: Analysis +type: bug +issues: + - 121441 From baca84c336a20631239aebff9d52562be4ea673a Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 17:21:16 +0200 Subject: [PATCH 13/36] Remove unnecessary checks for null --- .../org/elasticsearch/action/synonyms/PutSynonymsAction.java | 5 ----- 1 file changed, 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java index 91b1b3522cdb9..8c1e377b43aca 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java @@ -72,8 +72,6 @@ public Request(StreamInput in) throws IOException { } public Request(String synonymsSetId, TimeValue timeout, BytesReference content, XContentType contentType) throws IOException { - Objects.requireNonNull(timeout); - Objects.requireNonNull(synonymsSetId); this.synonymsSetId = synonymsSetId; this.timeout = timeout; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { @@ -84,9 +82,6 @@ public Request(String synonymsSetId, TimeValue timeout, BytesReference content, } Request(String synonymsSetId, SynonymRule[] synonymRules, TimeValue timeout) { - Objects.requireNonNull(timeout); - Objects.requireNonNull(synonymsSetId); - Objects.requireNonNull(synonymRules); this.synonymsSetId = synonymsSetId; this.synonymRules = synonymRules; this.timeout = timeout; From fbe393f4d6fc855fa1d3305a5028e89ad3b58454 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 17:31:20 +0200 Subject: [PATCH 14/36] Fix equals / HashCode --- .../action/synonyms/PutSynonymRuleAction.java | 12 ++++++------ .../action/synonyms/PutSynonymsAction.java | 2 +- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java index a5e49bbad8a06..6ec5666f0c13d 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java @@ -129,12 +129,12 @@ public TimeValue timeout() { @Override public boolean equals(Object o) { - if (o instanceof Request request) { - return timeout == request.timeout - && Objects.equals(synonymsSetId, request.synonymsSetId) - && Objects.equals(synonymRule, request.synonymRule); - } - return false; + if (this == o) return true; + if (o == null || getClass() != o.getClass()) return false; + Request request = (Request) o; + return Objects.equals(timeout, request.timeout) + && Objects.equals(synonymsSetId, request.synonymsSetId) + && Objects.equals(synonymRule, request.synonymRule); } @Override diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java index 8c1e377b43aca..a91e004b346e3 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java @@ -129,7 +129,7 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Request request = (Request) o; - return timeout == request.timeout + return Objects.equals(timeout, request.timeout) && Objects.equals(synonymsSetId, request.synonymsSetId) && Arrays.equals(synonymRules, request.synonymRules); } From a3dca505dfd07eaf15d3bf339e53bbf094fc6349 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 17:31:49 +0200 Subject: [PATCH 15/36] Checks that timeout is passed correctly to the check health method --- .../synonyms/SynonymsManagementAPIServiceIT.java | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 52b966922964e..5aa70587db1a9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -316,10 +316,13 @@ public void onFailure(Exception e) { public void testCreateSynonymsWithYellowSynonymsIndex() throws Exception { + TimeValue timeout = randomTimeValue(); + // Override health method check to simulate a timeout in checking the synonyms index synonymsManagementAPIService = new SynonymsManagementAPIService(client()) { @Override - void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { + void checkSynonymsIndexHealth(TimeValue actualTimeout, ActionListener listener) { + assertEquals(actualTimeout, timeout); ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); ClusterHealthResponse response = new ClusterHealthResponse( randomIdentifier(), @@ -334,7 +337,7 @@ void checkSynonymsIndexHealth(TimeValue timeout, ActionListener() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), timeout, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { fail("Shouldn't have been able to create synonyms with a timeout in synonyms index health"); @@ -356,7 +359,7 @@ public void onFailure(Exception e) { synonymsManagementAPIService.putSynonymRule( synonymSetId, randomSynonymRule(randomIdentifier()), - DEFAULT_TIMEOUT, + timeout, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { From 1ecfb12692d3950e8e1c369ff0c379e7acd1bb86 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 17:32:02 +0200 Subject: [PATCH 16/36] Use correctly the default timeout --- .../rest/action/synonyms/RestPutSynonymRuleAction.java | 3 ++- .../rest/action/synonyms/RestPutSynonymsAction.java | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java index b749366303154..28f0a42936bbb 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; +import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) @@ -42,7 +43,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient PutSynonymRuleAction.Request request = new PutSynonymRuleAction.Request( restRequest.param("synonymsSet"), restRequest.param("synonymRuleId"), - RestUtils.getAckTimeout(restRequest), + restRequest.paramAsTime(RestUtils.REST_TIMEOUT_PARAM, DEFAULT_TIMEOUT), restRequest.content(), restRequest.getXContentType() ); diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index a137234e9ed9c..e7faad478ac71 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -22,6 +22,7 @@ import java.io.IOException; import java.util.List; +import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) @@ -41,7 +42,7 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { PutSynonymsAction.Request request = new PutSynonymsAction.Request( restRequest.param("synonymsSet"), - RestUtils.getAckTimeout(restRequest), + restRequest.paramAsTime(RestUtils.REST_TIMEOUT_PARAM, DEFAULT_TIMEOUT), restRequest.content(), restRequest.getXContentType() ); From b419e4c7002c6174ea3e6d06a077b4d0fbf5a980 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 17:32:26 +0200 Subject: [PATCH 17/36] spotless --- .../SynonymsManagementAPIServiceIT.java | 29 ++++++++----------- 1 file changed, 12 insertions(+), 17 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 5aa70587db1a9..0e66d8be65dd7 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -356,25 +356,20 @@ public void onFailure(Exception e) { // Update a rule fails CountDownLatch updateLatch = new CountDownLatch(1); - synonymsManagementAPIService.putSynonymRule( - synonymSetId, - randomSynonymRule(randomIdentifier()), - timeout, - new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update synonyms with a timeout in synonyms index health"); - } + synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(randomIdentifier()), timeout, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to update synonyms with a timeout in synonyms index health"); + } - @Override - public void onFailure(Exception e) { - // Expected - assertTrue(e instanceof IndexCreationException); - assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); - updateLatch.countDown(); - } + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + updateLatch.countDown(); } - ); + }); updateLatch.await(5, TimeUnit.SECONDS); From eaeaaf7e3625f215045f7767b328d937e94e5113 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 18:22:01 +0200 Subject: [PATCH 18/36] Add monitor cluster privilege to internal synonyms user --- .../elasticsearch/xpack/core/security/user/InternalUsers.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java index 961f363be7958..f52739937dd19 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java @@ -33,6 +33,7 @@ import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ilm.action.ILMActions; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; +import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.support.MetadataUtils; import java.util.Arrays; @@ -283,7 +284,7 @@ public class InternalUsers { UsernamesField.SYNONYMS_USER_NAME, new RoleDescriptor( UsernamesField.SYNONYMS_ROLE_NAME, - null, + new String[] {"monitor"}, new RoleDescriptor.IndicesPrivileges[] { RoleDescriptor.IndicesPrivileges.builder().indices(".synonyms*").privileges("all").allowRestrictedIndices(true).build(), RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges(TransportReloadAnalyzersAction.TYPE.name()).build(), }, From 8fef96cf895cb5150485b43382dca8cf5bb8edf0 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Fri, 4 Apr 2025 16:30:11 +0000 Subject: [PATCH 19/36] [CI] Auto commit changes from spotless --- .../elasticsearch/xpack/core/security/user/InternalUsers.java | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java index f52739937dd19..3a378a1e6d895 100644 --- a/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java +++ b/x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/security/user/InternalUsers.java @@ -33,7 +33,6 @@ import org.elasticsearch.xpack.core.XPackPlugin; import org.elasticsearch.xpack.core.ilm.action.ILMActions; import org.elasticsearch.xpack.core.security.authz.RoleDescriptor; -import org.elasticsearch.xpack.core.security.authz.privilege.ConfigurableClusterPrivilege; import org.elasticsearch.xpack.core.security.support.MetadataUtils; import java.util.Arrays; @@ -284,7 +283,7 @@ public class InternalUsers { UsernamesField.SYNONYMS_USER_NAME, new RoleDescriptor( UsernamesField.SYNONYMS_ROLE_NAME, - new String[] {"monitor"}, + new String[] { "monitor" }, new RoleDescriptor.IndicesPrivileges[] { RoleDescriptor.IndicesPrivileges.builder().indices(".synonyms*").privileges("all").allowRestrictedIndices(true).build(), RoleDescriptor.IndicesPrivileges.builder().indices("*").privileges(TransportReloadAnalyzersAction.TYPE.name()).build(), }, From 8d3e221e25b6ba28984f3ffdf09f9e0d2dae45fb Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Fri, 4 Apr 2025 20:52:48 +0200 Subject: [PATCH 20/36] Add capabilities to avoid failing on bwc tests --- .../test/synonyms/10_synonyms_put.yml | 28 ++++++++++++------ .../test/synonyms/50_synonym_rule_put.yml | 29 ++++++++++++------- .../synonyms/RestPutSynonymRuleAction.java | 6 ++++ .../synonyms/RestPutSynonymsAction.java | 6 ++++ .../synonyms/SynonymPutCapabilities.java | 28 ++++++++++++++++++ 5 files changed, 78 insertions(+), 19 deletions(-) create mode 100644 server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml index bcb9ac3e4db9c..374d47940828f 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml @@ -67,6 +67,15 @@ setup: --- "Timeout can be specified": + + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: PUT + path: /_synonyms/{rule_id} + capabilities: [ put_synonyms_timeout ] + reason: "synonyms timeout capability needed" + - do: synonyms.put_synonym: id: test-update-synonyms @@ -81,6 +90,16 @@ setup: - match: { reload_analyzers_details._shards.total: 0 } - length: { reload_analyzers_details.reload_details: 0 } + # Validate timeout values + - do: + catch: /as a time value:\ negative durations are not supported/ + synonyms.put_synonym: + id: test-update-synonyms + timeout: -100s + body: + synonyms_set: + - synonyms: "bye, goodbye" + --- "Validation fails tests": - do: @@ -123,12 +142,3 @@ setup: synonyms_set: - synonyms: "bye, goodbye, " - - do: - catch: /as a time value:\ negative durations are not supported/ - synonyms.put_synonym: - id: test-update-synonyms - timeout: -100s - body: - synonyms_set: - - synonyms: "bye, goodbye" - diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml index abeb3078daed2..4bda6f4bc0c81 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml @@ -82,6 +82,14 @@ setup: --- "Timeout can be specified": + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: PUT + path: /_synonyms/{set_id}/{rule_id} + capabilities: [ put_synonyms_timeout ] + reason: "synonyms timeout capability needed" + - do: synonyms.put_synonym_rule: set_id: "test-synonyms" @@ -94,6 +102,17 @@ setup: - match: { reload_analyzers_details._shards.total: 0 } - length: { reload_analyzers_details.reload_details: 0 } + # Validates timeout values + - do: + catch: /as a time value:\ negative durations are not supported/ + synonyms.put_synonym_rule: + set_id: "test-synonyms" + rule_id: "test-id-0" + timeout: -100s + body: + synonyms_set: + - synonyms: "bye, goodbye" + --- "Validation failure tests": - do: @@ -135,13 +154,3 @@ setup: rule_id: "test-id-0" body: synonyms: "bye, goodbye, " - - - do: - catch: /as a time value:\ negative durations are not supported/ - synonyms.put_synonym_rule: - set_id: "test-synonyms" - rule_id: "test-id-0" - timeout: -100s - body: - synonyms_set: - - synonyms: "bye, goodbye" diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java index 28f0a42936bbb..0adae21c78ba0 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -53,4 +54,9 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient new RestToXContentListener<>(channel, SynonymUpdateResponse::status, r -> null) ); } + + @Override + public Set supportedCapabilities() { + return SynonymPutCapabilities.CAPABILITIES; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index e7faad478ac71..e15c23987fd88 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -21,6 +21,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @@ -52,4 +53,9 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient new RestToXContentListener<>(channel, SynonymUpdateResponse::status, r -> null) ); } + + @Override + public Set supportedCapabilities() { + return SynonymPutCapabilities.CAPABILITIES; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java new file mode 100644 index 0000000000000..4d59480756625 --- /dev/null +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java @@ -0,0 +1,28 @@ +/* + * 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.rest.action.synonyms; + +import java.util.Set; + +/** + * A {@link Set} of "capabilities" supported by the {@link RestPutSynonymsAction} and {@link RestPutSynonymRuleAction}. + */ +public final class SynonymPutCapabilities { + + private SynonymPutCapabilities() { + throw new UnsupportedOperationException("This is a utility class and cannot be instantiated"); + } + + private static final String PUT_SYNONYMS_TIMEOUT = "put_synonyms_timeout"; + + public static final Set CAPABILITIES = Set.of( + PUT_SYNONYMS_TIMEOUT + ); +} From 535a426dcd36be2a12507bbe3fc5589ef4061797 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 17:26:10 +0200 Subject: [PATCH 21/36] Replace timeout for refresh param --- .../SynonymsManagementAPIServiceIT.java | 222 ++++++++++-------- .../org/elasticsearch/TransportVersions.java | 2 +- .../synonyms/DeleteSynonymRuleAction.java | 18 +- .../action/synonyms/PutSynonymRuleAction.java | 33 ++- .../action/synonyms/PutSynonymsAction.java | 31 ++- .../synonyms/SynonymUpdateResponse.java | 14 +- .../TransportDeleteSynonymRuleAction.java | 1 + .../TransportPutSynonymRuleAction.java | 2 +- .../synonyms/TransportPutSynonymsAction.java | 2 +- .../synonyms/RestDeleteSynonymRuleAction.java | 9 +- .../synonyms/RestPutSynonymRuleAction.java | 6 +- .../synonyms/RestPutSynonymsAction.java | 6 +- ...bilities.java => SynonymCapabilities.java} | 10 +- .../SynonymsManagementAPIService.java | 110 +++++---- ...onymRuleActionRequestSerializingTests.java | 2 +- ...onymRuleActionRequestSerializingTests.java | 2 +- ...SynonymsActionRequestSerializingTests.java | 2 +- 17 files changed, 252 insertions(+), 220 deletions(-) rename server/src/main/java/org/elasticsearch/rest/action/synonyms/{SynonymPutCapabilities.java => SynonymCapabilities.java} (75%) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 0e66d8be65dd7..98564d3a58106 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -15,7 +15,6 @@ import org.elasticsearch.action.bulk.BulkResponse; import org.elasticsearch.cluster.ClusterName; import org.elasticsearch.cluster.ClusterState; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.index.mapper.extras.MapperExtrasPlugin; import org.elasticsearch.indices.IndexCreationException; import org.elasticsearch.plugins.Plugin; @@ -28,7 +27,6 @@ import java.util.concurrent.CountDownLatch; import java.util.concurrent.TimeUnit; -import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.action.synonyms.SynonymsTestUtils.randomSynonymRule; import static org.elasticsearch.action.synonyms.SynonymsTestUtils.randomSynonymsSet; import static org.mockito.ArgumentMatchers.anyString; @@ -56,11 +54,12 @@ public void setUp() throws Exception { public void testCreateManySynonyms() throws Exception { CountDownLatch putLatch = new CountDownLatch(1); String synonymSetId = randomIdentifier(); + boolean refresh = randomBoolean(); int rulesNumber = randomIntBetween(maxSynonymSets / 2, maxSynonymSets); synonymsManagementAPIService.putSynonymsSet( synonymSetId, randomSynonymsSet(rulesNumber, rulesNumber), - DEFAULT_TIMEOUT, + refresh, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { @@ -68,6 +67,7 @@ public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonym SynonymsManagementAPIService.UpdateSynonymsResultStatus.CREATED, synonymsReloadResult.synonymsOperationResult() ); + assertEquals(refresh, synonymsReloadResult.reloadAnalyzersResponse() != null); putLatch.countDown(); } @@ -106,7 +106,7 @@ public void testCreateTooManySynonymsAtOnce() throws InterruptedException { synonymsManagementAPIService.putSynonymsSet( randomIdentifier(), randomSynonymsSet(maxSynonymSets + 1, maxSynonymSets * 2), - DEFAULT_TIMEOUT, + randomBoolean(), new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { @@ -135,70 +135,69 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept synonymsManagementAPIService.putSynonymsSet( synonymSetId, randomSynonymsSet(synonymsToCreate), - DEFAULT_TIMEOUT, + randomBoolean(), new ActionListener<>() { @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Create as many rules as should fail - SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); - CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); - for (int i = 0; i < rulesToUpdate; i++) { - synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], DEFAULT_TIMEOUT, new ActionListener<>() { + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Create as many rules as should fail + SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); + CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); + for (int i = 0; i < rulesToUpdate; i++) { + synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], true, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + updatedRulesLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); + } + try { + updatedRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } + + // Updating more rules fails + int rulesToInsert = rules.length - rulesToUpdate; + CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); + for (int i = rulesToUpdate; i < rulesToInsert; i++) { + synonymsManagementAPIService.putSynonymRule( + // Error here + synonymSetId, + rules[i], + randomBoolean(), + new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - updatedRulesLatch.countDown(); + fail("Shouldn't have been able to update a rule"); } @Override public void onFailure(Exception e) { - fail(e); - } - }); - } - try { - updatedRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - fail(e); - } - - // Updating more rules fails - int rulesToInsert = rules.length - rulesToUpdate; - CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); - for (int i = rulesToUpdate; i < rulesToInsert; i++) { - synonymsManagementAPIService.putSynonymRule( - // Error here - synonymSetId, - rules[i], - DEFAULT_TIMEOUT, - new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update a rule"); - } - - @Override - public void onFailure(Exception e) { - if (e instanceof IllegalArgumentException == false) { - fail(e); - } - updatedRulesLatch.countDown(); + if (e instanceof IllegalArgumentException == false) { + fail(e); } + updatedRulesLatch.countDown(); } - ); - } - try { - insertRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - fail(e); - } + } + ); } - - @Override - public void onFailure(Exception e) { + try { + insertRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { fail(e); } } - ); + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); latch.await(5, TimeUnit.SECONDS); } @@ -207,14 +206,14 @@ public void testUpdateRuleWithMaxSynonyms() throws InterruptedException { CountDownLatch latch = new CountDownLatch(1); String synonymSetId = randomIdentifier(); SynonymRule[] synonymsSet = randomSynonymsSet(maxSynonymSets, maxSynonymSets); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, DEFAULT_TIMEOUT, new ActionListener<>() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails synonymsManagementAPIService.putSynonymRule( synonymSetId, synonymsSet[randomIntBetween(0, maxSynonymSets - 1)], - DEFAULT_TIMEOUT, + randomBoolean(), new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { @@ -243,26 +242,21 @@ public void testCreateRuleWithMaxSynonyms() throws InterruptedException { String synonymSetId = randomIdentifier(); String ruleId = randomIdentifier(); SynonymRule[] synonymsSet = randomSynonymsSet(maxSynonymSets, maxSynonymSets); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, DEFAULT_TIMEOUT, new ActionListener<>() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, synonymsSet, randomBoolean(), new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Updating a rule fails - synonymsManagementAPIService.putSynonymRule( - synonymSetId, - randomSynonymRule(ruleId), - DEFAULT_TIMEOUT, - new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Should not create a new rule that does not exist when at max capacity"); - } + synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(ruleId), true, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Should not create a new rule that does not exist when at max capacity"); + } - @Override - public void onFailure(Exception e) { - latch.countDown(); - } + @Override + public void onFailure(Exception e) { + latch.countDown(); } - ); + }); } @Override @@ -316,13 +310,10 @@ public void onFailure(Exception e) { public void testCreateSynonymsWithYellowSynonymsIndex() throws Exception { - TimeValue timeout = randomTimeValue(); - // Override health method check to simulate a timeout in checking the synonyms index synonymsManagementAPIService = new SynonymsManagementAPIService(client()) { @Override - void checkSynonymsIndexHealth(TimeValue actualTimeout, ActionListener listener) { - assertEquals(actualTimeout, timeout); + void checkSynonymsIndexHealth(ActionListener listener) { ClusterState clusterState = ClusterState.builder(ClusterName.DEFAULT).build(); ClusterHealthResponse response = new ClusterHealthResponse( randomIdentifier(), @@ -337,10 +328,10 @@ void checkSynonymsIndexHealth(TimeValue actualTimeout, ActionListener() { + synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to create synonyms with a timeout in synonyms index health"); + fail("Shouldn't have been able to create synonyms with refresh in synonyms index health"); } @Override @@ -356,10 +347,11 @@ public void onFailure(Exception e) { // Update a rule fails CountDownLatch updateLatch = new CountDownLatch(1); - synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(randomIdentifier()), timeout, new ActionListener<>() { + String synonymRuleId = randomIdentifier(); + synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(synonymRuleId), true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update synonyms with a timeout in synonyms index health"); + fail("Shouldn't have been able to update synonyms with refresh in synonyms index health"); } @Override @@ -373,9 +365,28 @@ public void onFailure(Exception e) { updateLatch.await(5, TimeUnit.SECONDS); - // But, we can still create a synonyms set with timeout -1 - CountDownLatch putWithoutTimeoutLatch = new CountDownLatch(1); - synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), TimeValue.MINUS_ONE, new ActionListener<>() { + // Delete a rule fails + CountDownLatch deleteLatch = new CountDownLatch(1); + synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, true, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to delete a synonym rule with refresh in synonyms index health"); + } + + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + updateLatch.countDown(); + } + }); + + deleteLatch.await(5, TimeUnit.SECONDS); + + // But, we can still create a synonyms set without refresh + CountDownLatch putNoRefreshLatch = new CountDownLatch(1); + synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), false, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { // Expected @@ -388,28 +399,39 @@ public void onFailure(Exception e) { } }); - putWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); + putNoRefreshLatch.await(5, TimeUnit.SECONDS); // Same for update - CountDownLatch putRuleWithoutTimeoutLatch = new CountDownLatch(1); - synonymsManagementAPIService.putSynonymRule( - synonymSetId, - randomSynonymRule(randomIdentifier()), - TimeValue.MINUS_ONE, - new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Expected - putRuleWithoutTimeoutLatch.countDown(); - } + CountDownLatch putRuleNoRefreshLatch = new CountDownLatch(1); + synonymsManagementAPIService.putSynonymRule(synonymSetId, randomSynonymRule(synonymRuleId), false, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Expected + putRuleNoRefreshLatch.countDown(); + } - @Override - public void onFailure(Exception e) { - fail(e); - } + @Override + public void onFailure(Exception e) { + fail(e); } - ); + }); + + putRuleNoRefreshLatch.await(5, TimeUnit.SECONDS); + + // Same for delete + CountDownLatch deleteNoRefreshLatch = new CountDownLatch(1); + synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, false, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + deleteNoRefreshLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); - putRuleWithoutTimeoutLatch.await(5, TimeUnit.SECONDS); + deleteNoRefreshLatch.await(5, TimeUnit.SECONDS); } } diff --git a/server/src/main/java/org/elasticsearch/TransportVersions.java b/server/src/main/java/org/elasticsearch/TransportVersions.java index c10784bfec842..eada57b5705af 100644 --- a/server/src/main/java/org/elasticsearch/TransportVersions.java +++ b/server/src/main/java/org/elasticsearch/TransportVersions.java @@ -214,7 +214,7 @@ static TransportVersion def(int id) { public static final TransportVersion ESQL_REMOVE_AGGREGATE_TYPE = def(9_045_0_00); public static final TransportVersion ADD_PROJECT_ID_TO_DSL_ERROR_INFO = def(9_046_0_00); public static final TransportVersion SEMANTIC_TEXT_CHUNKING_CONFIG = def(9_047_00_0); - public static final TransportVersion SYNONYMS_UPDATE_TIMEOUT = def(9_048_0_00); + public static final TransportVersion SYNONYMS_REFRESH_PARAM = def(9_048_0_00); /* * STOP! READ THIS FIRST! No, really, diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java index 9f52f1552e421..a485ed19ac742 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; @@ -31,18 +32,24 @@ public DeleteSynonymRuleAction() { public static class Request extends ActionRequest { private final String synonymsSetId; - private final String synonymRuleId; + private final boolean refresh; public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRuleId = in.readString(); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + this.refresh = in.readBoolean(); + } else { + this.refresh = true; + } } - public Request(String synonymsSetId, String synonymRuleId) { + public Request(String synonymsSetId, String synonymRuleId, boolean refresh) { this.synonymsSetId = synonymsSetId; this.synonymRuleId = synonymRuleId; + this.refresh = refresh; } @Override @@ -63,6 +70,9 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); out.writeString(synonymRuleId); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + out.writeBoolean(refresh); + } } public String synonymsSetId() { @@ -73,6 +83,10 @@ public String synonymRuleId() { return synonymRuleId; } + public boolean refresh() { + return refresh; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java index 6ec5666f0c13d..a504ecdc6f8ad 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymRuleAction.java @@ -19,7 +19,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.synonyms.SynonymRule; import org.elasticsearch.synonyms.SynonymsManagementAPIService; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -36,8 +35,6 @@ public class PutSynonymRuleAction extends ActionType { public static final PutSynonymRuleAction INSTANCE = new PutSynonymRuleAction(); public static final String NAME = "cluster:admin/synonym_rules/put"; - public static final TimeValue DEFAULT_TIMEOUT = TimeValue.timeValueSeconds(10); - public PutSynonymRuleAction() { super(NAME); } @@ -45,7 +42,7 @@ public PutSynonymRuleAction() { public static class Request extends ActionRequest { private final String synonymsSetId; private final SynonymRule synonymRule; - private final TimeValue timeout; + private final boolean refresh; public static final ParseField SYNONYMS_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( @@ -62,30 +59,28 @@ public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRule = new SynonymRule(in); - if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - this.timeout = in.readTimeValue(); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + this.refresh = in.readBoolean(); } else { - this.timeout = DEFAULT_TIMEOUT; + this.refresh = true; } } - public Request(String synonymsSetId, String synonymRuleId, TimeValue timeout, BytesReference content, XContentType contentType) + public Request(String synonymsSetId, String synonymRuleId, boolean refresh, BytesReference content, XContentType contentType) throws IOException { - Objects.requireNonNull(timeout); this.synonymsSetId = synonymsSetId; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { this.synonymRule = PARSER.apply(parser, synonymRuleId); } catch (Exception e) { throw new IllegalArgumentException("Failed to parse: " + content.utf8ToString(), e); } - this.timeout = timeout; + this.refresh = refresh; } - Request(String synonymsSetId, SynonymRule synonymRule, TimeValue timeout) { - Objects.requireNonNull(timeout); + Request(String synonymsSetId, SynonymRule synonymRule, boolean refresh) { this.synonymsSetId = synonymsSetId; this.synonymRule = synonymRule; - this.timeout = timeout; + this.refresh = refresh; } @Override @@ -110,8 +105,8 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); synonymRule.writeTo(out); - if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - out.writeTimeValue(timeout); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + out.writeBoolean(refresh); } } @@ -123,8 +118,8 @@ public SynonymRule synonymRule() { return synonymRule; } - public TimeValue timeout() { - return timeout; + public boolean refresh() { + return refresh; } @Override @@ -132,14 +127,14 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Request request = (Request) o; - return Objects.equals(timeout, request.timeout) + return Objects.equals(refresh, request.refresh) && Objects.equals(synonymsSetId, request.synonymsSetId) && Objects.equals(synonymRule, request.synonymRule); } @Override public int hashCode() { - return Objects.hash(synonymsSetId, synonymRule, timeout); + return Objects.hash(synonymsSetId, synonymRule, refresh); } } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java index a91e004b346e3..1b9b6b36cb2f8 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java @@ -19,7 +19,6 @@ import org.elasticsearch.common.io.stream.StreamInput; import org.elasticsearch.common.io.stream.StreamOutput; import org.elasticsearch.common.xcontent.XContentHelper; -import org.elasticsearch.core.TimeValue; import org.elasticsearch.synonyms.SynonymRule; import org.elasticsearch.synonyms.SynonymsManagementAPIService; import org.elasticsearch.xcontent.ConstructingObjectParser; @@ -33,8 +32,6 @@ import java.util.List; import java.util.Objects; -import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; - public class PutSynonymsAction extends ActionType { public static final PutSynonymsAction INSTANCE = new PutSynonymsAction(); @@ -47,7 +44,7 @@ public PutSynonymsAction() { public static class Request extends ActionRequest { private final String synonymsSetId; private final SynonymRule[] synonymRules; - private final TimeValue timeout; + private final boolean refresh; public static final ParseField SYNONYMS_SET_FIELD = new ParseField(SynonymsManagementAPIService.SYNONYMS_SET_FIELD); private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>("synonyms_set", args -> { @@ -64,16 +61,16 @@ public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRules = in.readArray(SynonymRule::new, SynonymRule[]::new); - if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - this.timeout = in.readTimeValue(); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + this.refresh = in.readBoolean(); } else { - this.timeout = DEFAULT_TIMEOUT; + this.refresh = false; } } - public Request(String synonymsSetId, TimeValue timeout, BytesReference content, XContentType contentType) throws IOException { + public Request(String synonymsSetId, boolean refresh, BytesReference content, XContentType contentType) throws IOException { this.synonymsSetId = synonymsSetId; - this.timeout = timeout; + this.refresh = refresh; try (XContentParser parser = XContentHelper.createParser(XContentParserConfiguration.EMPTY, content, contentType)) { this.synonymRules = PARSER.apply(parser, null); } catch (Exception e) { @@ -81,10 +78,10 @@ public Request(String synonymsSetId, TimeValue timeout, BytesReference content, } } - Request(String synonymsSetId, SynonymRule[] synonymRules, TimeValue timeout) { + Request(String synonymsSetId, SynonymRule[] synonymRules, boolean refresh) { this.synonymsSetId = synonymsSetId; this.synonymRules = synonymRules; - this.timeout = timeout; + this.refresh = refresh; } @Override @@ -107,8 +104,8 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); out.writeArray(synonymRules); - if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_UPDATE_TIMEOUT)) { - out.writeTimeValue(timeout); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + out.writeBoolean(refresh); } } @@ -116,8 +113,8 @@ public String synonymsSetId() { return synonymsSetId; } - public TimeValue timeout() { - return timeout; + public boolean refresh() { + return refresh; } public SynonymRule[] synonymRules() { @@ -129,14 +126,14 @@ public boolean equals(Object o) { if (this == o) return true; if (o == null || getClass() != o.getClass()) return false; Request request = (Request) o; - return Objects.equals(timeout, request.timeout) + return Objects.equals(refresh, request.refresh) && Objects.equals(synonymsSetId, request.synonymsSetId) && Arrays.equals(synonymRules, request.synonymRules); } @Override public int hashCode() { - return Objects.hash(synonymsSetId, Arrays.hashCode(synonymRules), timeout); + return Objects.hash(synonymsSetId, Arrays.hashCode(synonymRules), refresh); } } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java b/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java index c1a9f2a80c470..cd47491a13335 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionResponse; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse; import org.elasticsearch.common.io.stream.StreamInput; @@ -30,7 +31,11 @@ public class SynonymUpdateResponse extends ActionResponse implements ToXContentO public SynonymUpdateResponse(StreamInput in) throws IOException { this.updateStatus = in.readEnum(UpdateSynonymsResultStatus.class); - this.reloadAnalyzersResponse = new ReloadAnalyzersResponse(in); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + this.reloadAnalyzersResponse = in.readOptionalWriteable(ReloadAnalyzersResponse::new); + } else { + this.reloadAnalyzersResponse = new ReloadAnalyzersResponse(in); + } } public SynonymUpdateResponse(SynonymsReloadResult synonymsReloadResult) { @@ -38,7 +43,6 @@ public SynonymUpdateResponse(SynonymsReloadResult synonymsReloadResult) { UpdateSynonymsResultStatus updateStatus = synonymsReloadResult.synonymsOperationResult(); Objects.requireNonNull(updateStatus, "Update status must not be null"); ReloadAnalyzersResponse reloadResponse = synonymsReloadResult.reloadAnalyzersResponse(); - Objects.requireNonNull(reloadResponse, "Reload analyzers response must not be null"); this.updateStatus = updateStatus; this.reloadAnalyzersResponse = reloadResponse; @@ -49,8 +53,10 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws builder.startObject(); { builder.field("result", updateStatus.name().toLowerCase(Locale.ENGLISH)); - builder.field("reload_analyzers_details"); - reloadAnalyzersResponse.toXContent(builder, params); + if (reloadAnalyzersResponse != null) { + builder.field("reload_analyzers_details"); + reloadAnalyzersResponse.toXContent(builder, params); + } } builder.endObject(); diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java index 0d55774489c4c..9fed5e5716c5b 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java @@ -41,6 +41,7 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act synonymsManagementAPIService.deleteSynonymRule( request.synonymsSetId(), request.synonymRuleId(), + request.refresh(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java index c9e24dfd78e8e..6cd2d74b9f315 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymRuleAction.java @@ -41,7 +41,7 @@ protected void doExecute(Task task, PutSynonymRuleAction.Request request, Action synonymsManagementAPIService.putSynonymRule( request.synonymsSetId(), request.synonymRule(), - request.timeout(), + request.refresh(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java index 8a738b0ba65b8..290f62e7bad57 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java @@ -35,7 +35,7 @@ protected void doExecute(Task task, PutSynonymsAction.Request request, ActionLis synonymsManagementAPIService.putSynonymsSet( request.synonymsSetId(), request.synonymRules(), - request.timeout(), + request.refresh(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java index b65dd6bc95865..9aa8265ce35b6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java @@ -19,6 +19,7 @@ import java.io.IOException; import java.util.List; +import java.util.Set; import static org.elasticsearch.rest.RestRequest.Method.DELETE; @@ -39,8 +40,14 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { DeleteSynonymRuleAction.Request request = new DeleteSynonymRuleAction.Request( restRequest.param("synonymsSet"), - restRequest.param("synonymRuleId") + restRequest.param("synonymRuleId"), + restRequest.paramAsBoolean("refresh", true) ); return channel -> client.execute(DeleteSynonymRuleAction.INSTANCE, request, new RestToXContentListener<>(channel)); } + + @Override + public Set supportedCapabilities() { + return SynonymCapabilities.CAPABILITIES; + } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java index 0adae21c78ba0..0af0dad3696eb 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymRuleAction.java @@ -14,7 +14,6 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; @@ -23,7 +22,6 @@ import java.util.List; import java.util.Set; -import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) @@ -44,7 +42,7 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient PutSynonymRuleAction.Request request = new PutSynonymRuleAction.Request( restRequest.param("synonymsSet"), restRequest.param("synonymRuleId"), - restRequest.paramAsTime(RestUtils.REST_TIMEOUT_PARAM, DEFAULT_TIMEOUT), + restRequest.paramAsBoolean("refresh", true), restRequest.content(), restRequest.getXContentType() ); @@ -57,6 +55,6 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient @Override public Set supportedCapabilities() { - return SynonymPutCapabilities.CAPABILITIES; + return SynonymCapabilities.CAPABILITIES; } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java index e15c23987fd88..46a63a12d0e0b 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestPutSynonymsAction.java @@ -14,7 +14,6 @@ import org.elasticsearch.client.internal.node.NodeClient; import org.elasticsearch.rest.BaseRestHandler; import org.elasticsearch.rest.RestRequest; -import org.elasticsearch.rest.RestUtils; import org.elasticsearch.rest.Scope; import org.elasticsearch.rest.ServerlessScope; import org.elasticsearch.rest.action.RestToXContentListener; @@ -23,7 +22,6 @@ import java.util.List; import java.util.Set; -import static org.elasticsearch.action.synonyms.PutSynonymRuleAction.DEFAULT_TIMEOUT; import static org.elasticsearch.rest.RestRequest.Method.PUT; @ServerlessScope(Scope.PUBLIC) @@ -43,7 +41,7 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { PutSynonymsAction.Request request = new PutSynonymsAction.Request( restRequest.param("synonymsSet"), - restRequest.paramAsTime(RestUtils.REST_TIMEOUT_PARAM, DEFAULT_TIMEOUT), + restRequest.paramAsBoolean("refresh", true), restRequest.content(), restRequest.getXContentType() ); @@ -56,6 +54,6 @@ protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient @Override public Set supportedCapabilities() { - return SynonymPutCapabilities.CAPABILITIES; + return SynonymCapabilities.CAPABILITIES; } } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymCapabilities.java similarity index 75% rename from server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java rename to server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymCapabilities.java index 4d59480756625..6700657392705 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymPutCapabilities.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/SynonymCapabilities.java @@ -14,15 +14,13 @@ /** * A {@link Set} of "capabilities" supported by the {@link RestPutSynonymsAction} and {@link RestPutSynonymRuleAction}. */ -public final class SynonymPutCapabilities { +public final class SynonymCapabilities { - private SynonymPutCapabilities() { + private SynonymCapabilities() { throw new UnsupportedOperationException("This is a utility class and cannot be instantiated"); } - private static final String PUT_SYNONYMS_TIMEOUT = "put_synonyms_timeout"; + private static final String SYNONYMS_REFRESH_PARAM = "synonyms_refresh_param"; - public static final Set CAPABILITIES = Set.of( - PUT_SYNONYMS_TIMEOUT - ); + public static final Set CAPABILITIES = Set.of(SYNONYMS_REFRESH_PARAM); } diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 57658a014e72e..8619b11dac0a1 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -95,6 +95,7 @@ public class SynonymsManagementAPIService { private static final String SYNONYM_RULE_ID_FIELD = SynonymRule.ID_FIELD.getPreferredName(); private static final String SYNONYM_SETS_AGG_NAME = "synonym_sets_aggr"; private static final int SYNONYMS_INDEX_MAPPINGS_VERSION = 1; + public static final int INDEX_SEARCHABLE_TIMEOUT_SECONDS = 30; private final int maxSynonymsSets; // Package private for testing @@ -309,7 +310,7 @@ private static void logUniqueFailureMessagesWithIndices(List listener ) { if (synonymsSet.length > maxSynonymsSets) { @@ -353,13 +354,7 @@ public void putSynonymsSet( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - ensureSynonymsSearchableAndReloadAnalyzers( - synonymSetId, - false, - bulkInsertResponseListener, - updateSynonymsResultStatus, - timeout - ); + maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); }) ); })); @@ -385,7 +380,7 @@ void bulkUpdateSynonymsSet(String synonymSetId, SynonymRule[] synonymsSet, Actio public void putSynonymRule( String synonymsSetId, SynonymRule synonymRule, - TimeValue timeout, + boolean refresh, ActionListener listener ) { checkSynonymSetExists(synonymsSetId, listener.delegateFailureAndWrap((l1, obj) -> { @@ -409,7 +404,7 @@ public void putSynonymRule( new IllegalArgumentException("The number of synonym rules in a synonyms set cannot exceed " + maxSynonymsSets) ); } else { - indexSynonymRule(synonymsSetId, synonymRule, timeout, searchListener); + indexSynonymRule(synonymsSetId, synonymRule, refresh, searchListener); } })); })); @@ -418,7 +413,7 @@ public void putSynonymRule( private void indexSynonymRule( String synonymsSetId, SynonymRule synonymRule, - TimeValue timeout, + boolean refresh, ActionListener listener ) throws IOException { IndexRequest indexRequest = createSynonymRuleIndexRequest(synonymsSetId, synonymRule).setRefreshPolicy( @@ -429,7 +424,7 @@ private void indexSynonymRule( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - ensureSynonymsSearchableAndReloadAnalyzers(synonymsSetId, false, l2, updateStatus, timeout); + maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); })); } @@ -449,7 +444,12 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList ); } - public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener) { + public void deleteSynonymRule( + String synonymsSetId, + String synonymRuleId, + boolean refresh, + ActionListener listener + ) { client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId)) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> { @@ -468,7 +468,7 @@ public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, Action return; } - reloadAnalyzers(synonymsSetId, false, listener, UpdateSynonymsResultStatus.DELETED); + maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener); })); } @@ -526,7 +526,7 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener) { // Previews reloading the resource to understand its usage on indices - reloadAnalyzers(synonymSetId, true, listener.delegateFailure((reloadListener, reloadResult) -> { + maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -563,61 +563,57 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void ensureSynonymsSearchableAndReloadAnalyzers( + private void maybeReloadAnalyzers( String synonymSetId, + boolean refresh, boolean preview, - ActionListener listener, UpdateSynonymsResultStatus synonymsOperationResult, - TimeValue timeout + ActionListener listener ) { - // Ensure synonyms index is searchable if timeout is present - if (TimeValue.MINUS_ONE.equals(timeout) == false) { - checkSynonymsIndexHealth(timeout, listener.delegateFailure((l, response) -> { - if (response.isTimedOut()) { - l.onFailure( - new IndexCreationException( - "synonyms index [" - + SYNONYMS_ALIAS_NAME - + "] is not searchable. " - + response.getActiveShardsPercent() - + "% shards are active", - null - ) - ); - return; - } - reloadAnalyzers(synonymSetId, preview, l, synonymsOperationResult); - })); - } else { - reloadAnalyzers(synonymSetId, preview, listener, synonymsOperationResult); + if (refresh == false) { + // If not refreshing, we don't need to reload analyzers + listener.onResponse(new SynonymsReloadResult(synonymsOperationResult, null)); + return; } - } - // Allows checking failures in tests - void checkSynonymsIndexHealth(TimeValue timeout, ActionListener listener) { - ClusterHealthRequest healthRequest = new ClusterHealthRequest(timeout, SYNONYMS_ALIAS_NAME).waitForGreenStatus(); + // Check synonyms index is searchable before reloading, to ensure analyzers are able to load the changed information + checkSynonymsIndexHealth(listener.delegateFailure((l, response) -> { + if (response.isTimedOut()) { + l.onFailure( + new IndexCreationException( + "synonyms index [" + + SYNONYMS_ALIAS_NAME + + "] is not searchable. " + + response.getActiveShardsPercent() + + "% shards are active", + null + ) + ); + return; + } - client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener); + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); + client.execute( + TransportReloadAnalyzersAction.TYPE, + reloadAnalyzersRequest, + listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) + ); + })); } - private void reloadAnalyzers( - String synonymSetId, - boolean preview, - ActionListener listener, - UpdateSynonymsResultStatus synonymsOperationResult - ) { + // Allows checking failures in tests + void checkSynonymsIndexHealth(ActionListener listener) { + ClusterHealthRequest healthRequest = new ClusterHealthRequest( + TimeValue.timeValueSeconds(INDEX_SEARCHABLE_TIMEOUT_SECONDS), + SYNONYMS_ALIAS_NAME + ).waitForGreenStatus(); - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) - ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); - client.execute( - TransportReloadAnalyzersAction.TYPE, - reloadAnalyzersRequest, - listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) - ); + client.execute(TransportClusterHealthAction.TYPE, healthRequest, listener); } // Retrieves the internal synonym rule ID to store it in the index. As the same synonym rule ID diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java index 68adf4034ce51..97ca4e22869e1 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java @@ -23,7 +23,7 @@ protected Writeable.Reader instanceReader() { @Override protected DeleteSynonymRuleAction.Request createTestInstance() { - return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier()); + return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier(), randomBoolean()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java index db7c056557168..1c5a15d303470 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymRuleActionRequestSerializingTests.java @@ -22,7 +22,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymRuleAction.Request createTestInstance() { - return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule(), randomTimeValue()); + return new PutSynonymRuleAction.Request(randomIdentifier(), SynonymsTestUtils.randomSynonymRule(), randomBoolean()); } @Override diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java index 9c67f32596c47..ced6e18e9ccad 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionRequestSerializingTests.java @@ -25,7 +25,7 @@ protected Writeable.Reader instanceReader() { @Override protected PutSynonymsAction.Request createTestInstance() { - return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet(), randomTimeValue()); + return new PutSynonymsAction.Request(randomIdentifier(), randomSynonymsSet(), randomBoolean()); } @Override From f36056f9ef438b602af6f71c0c689b1cf731c62e Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 17:30:35 +0200 Subject: [PATCH 22/36] Add param to specs --- .../rest-api-spec/api/synonyms.delete_synonym_rule.json | 6 ++++++ .../resources/rest-api-spec/api/synonyms.put_synonym.json | 6 +++--- .../rest-api-spec/api/synonyms.put_synonym_rule.json | 6 +++--- 3 files changed, 12 insertions(+), 6 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json index 5a0de4ab94a7c..e2285bbd6d4ae 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json @@ -33,6 +33,12 @@ } } ] + }, + "params": { + "refresh": { + "type": "boolean", + "description": "Refresh search analyzers to update synonyms" + } } } } diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json index 2ea7b34afc8a4..3e700163e1738 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym.json @@ -31,9 +31,9 @@ ] }, "params": { - "timeout": { - "type": "time", - "description": "Explicit timeout for the operation to complete" + "refresh": { + "type": "boolean", + "description": "Refresh search analyzers to update synonyms" } }, "body": { diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json index c4bd5ba46c50f..55edd65a8beb2 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.put_synonym_rule.json @@ -35,9 +35,9 @@ ] }, "params": { - "timeout": { - "type": "time", - "description": "Explicit timeout for the operation to complete" + "refresh": { + "type": "boolean", + "description": "Refresh search analyzers to update synonyms" } }, "body": { From 5a8209aac2f493926af3e0093c4698202dc99a43 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 17:30:42 +0200 Subject: [PATCH 23/36] Add YAML tests --- .../test/synonyms/10_synonyms_put.yml | 22 ++++--------- .../test/synonyms/50_synonym_rule_put.yml | 32 +++++++------------ .../test/synonyms/70_synonym_rule_delete.yml | 21 ++++++++++++ 3 files changed, 38 insertions(+), 37 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml index 374d47940828f..69b04e92d1d5f 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/10_synonyms_put.yml @@ -66,20 +66,20 @@ setup: - match: { synonyms_set: [] } --- -"Timeout can be specified": +"Refresh can be specified": - requires: test_runner_features: [ capabilities ] capabilities: - method: PUT path: /_synonyms/{rule_id} - capabilities: [ put_synonyms_timeout ] - reason: "synonyms timeout capability needed" + capabilities: [ synonyms_refresh_param ] + reason: "synonyms refresh param capability needed" - do: synonyms.put_synonym: id: test-update-synonyms - timeout: 10s + refresh: false body: synonyms_set: - synonyms: "hello, hi" @@ -87,18 +87,8 @@ setup: id: "test-id" - match: { result: "created" } - - match: { reload_analyzers_details._shards.total: 0 } - - length: { reload_analyzers_details.reload_details: 0 } - - # Validate timeout values - - do: - catch: /as a time value:\ negative durations are not supported/ - synonyms.put_synonym: - id: test-update-synonyms - timeout: -100s - body: - synonyms_set: - - synonyms: "bye, goodbye" + # Reload analyzers info is not included + - not_exists: reload_analyzers_details --- "Validation fails tests": diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml index 4bda6f4bc0c81..3e618176eed2c 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/50_synonym_rule_put.yml @@ -81,37 +81,27 @@ setup: synonyms: "i-phone, iphone" --- -"Timeout can be specified": +"Refresh can be specified": + - requires: test_runner_features: [ capabilities ] capabilities: - method: PUT - path: /_synonyms/{set_id}/{rule_id} - capabilities: [ put_synonyms_timeout ] - reason: "synonyms timeout capability needed" + path: /_synonyms/{rule_id} + capabilities: [ synonyms_refresh_param ] + reason: "synonyms refresh param capability needed" - do: synonyms.put_synonym_rule: + refresh: false set_id: "test-synonyms" - rule_id: "test-id-0" - timeout: 10s + rule_id: "test-id-2" body: - synonyms: "i-phone, iphone" - - - match: { result: "created" } - - match: { reload_analyzers_details._shards.total: 0 } - - length: { reload_analyzers_details.reload_details: 0 } + synonyms: "bye, goodbye, seeya" - # Validates timeout values - - do: - catch: /as a time value:\ negative durations are not supported/ - synonyms.put_synonym_rule: - set_id: "test-synonyms" - rule_id: "test-id-0" - timeout: -100s - body: - synonyms_set: - - synonyms: "bye, goodbye" + - match: { result: "updated" } + # Reload analyzers info is not included + - not_exists: reload_analyzers_details --- "Validation failure tests": diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml index a4853b0b6d414..50d36b5c5db63 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml @@ -44,6 +44,27 @@ setup: - synonyms: "test => check" id: "test-id-3" +--- +"Refresh can be specified": + + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: PUT + path: /_synonyms/{rule_id} + capabilities: [ synonyms_refresh_param ] + reason: "synonyms refresh param capability needed" + + - do: + synonyms.delete_synonym_rule: + set_id: test-synonyms + rule_id: test-id-2 + refresh: false + + - match: { result: "deleted" } + # Reload analyzers info is not included + - not_exists: reload_analyzers_details + --- "Delete synonym rule - missing synonym set": - do: From d2b95446f531b0918ee15d27a2da88bd10086b8d Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 17:40:43 +0200 Subject: [PATCH 24/36] Fix changelog --- docs/changelog/126314.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/changelog/126314.yaml b/docs/changelog/126314.yaml index 4336c3e39e10e..8d16788f8b0c0 100644 --- a/docs/changelog/126314.yaml +++ b/docs/changelog/126314.yaml @@ -1,5 +1,5 @@ pr: 126314 -summary: Add timeout to synonyms put APIs to wait for synonyms to be accessible +summary: Add refresh to synonyms put / delete APIs to wait for synonyms to be accessible and reload analyzers area: Analysis type: bug issues: From f8ee27af8077f7783458771114e66309f465785e Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 16 Apr 2025 15:50:24 +0000 Subject: [PATCH 25/36] [CI] Auto commit changes from spotless --- .../SynonymsManagementAPIServiceIT.java | 97 ++++++++++--------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 98564d3a58106..59dd19d8536e9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -138,66 +138,67 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept randomBoolean(), new ActionListener<>() { @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Create as many rules as should fail - SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); - CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); - for (int i = 0; i < rulesToUpdate; i++) { - synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], true, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - updatedRulesLatch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); - } - try { - updatedRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - fail(e); - } - - // Updating more rules fails - int rulesToInsert = rules.length - rulesToUpdate; - CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); - for (int i = rulesToUpdate; i < rulesToInsert; i++) { - synonymsManagementAPIService.putSynonymRule( - // Error here - synonymSetId, - rules[i], - randomBoolean(), - new ActionListener<>() { + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Create as many rules as should fail + SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); + CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); + for (int i = 0; i < rulesToUpdate; i++) { + synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update a rule"); + updatedRulesLatch.countDown(); } @Override public void onFailure(Exception e) { - if (e instanceof IllegalArgumentException == false) { - fail(e); + fail(e); + } + }); + } + try { + updatedRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } + + // Updating more rules fails + int rulesToInsert = rules.length - rulesToUpdate; + CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); + for (int i = rulesToUpdate; i < rulesToInsert; i++) { + synonymsManagementAPIService.putSynonymRule( + // Error here + synonymSetId, + rules[i], + randomBoolean(), + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to update a rule"); + } + + @Override + public void onFailure(Exception e) { + if (e instanceof IllegalArgumentException == false) { + fail(e); + } + updatedRulesLatch.countDown(); } - updatedRulesLatch.countDown(); } - } - ); + ); + } + try { + insertRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } } - try { - insertRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { + + @Override + public void onFailure(Exception e) { fail(e); } } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); + ); latch.await(5, TimeUnit.SECONDS); } From c6bcdbd77ab7f65da3963611357a1d59b6ff5389 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 18:51:43 +0200 Subject: [PATCH 26/36] Use BWC serialization tests --- .../synonyms/SynonymUpdateResponse.java | 29 ++++++- ...SynonymUpdateResponseSerializingTests.java | 79 +++++++++++++++++-- 2 files changed, 100 insertions(+), 8 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java b/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java index cd47491a13335..5c55bd54a00d4 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/SynonymUpdateResponse.java @@ -21,11 +21,17 @@ import org.elasticsearch.xcontent.XContentBuilder; import java.io.IOException; +import java.util.List; import java.util.Locale; +import java.util.Map; import java.util.Objects; public class SynonymUpdateResponse extends ActionResponse implements ToXContentObject { + public static final String RESULT_FIELD = "result"; + public static final String RELOAD_ANALYZERS_DETAILS_FIELD = "reload_analyzers_details"; + static final ReloadAnalyzersResponse EMPTY_RELOAD_ANALYZER_RESPONSE = new ReloadAnalyzersResponse(0, 0, 0, List.of(), Map.of()); + private final UpdateSynonymsResultStatus updateStatus; private final ReloadAnalyzersResponse reloadAnalyzersResponse; @@ -52,9 +58,9 @@ public SynonymUpdateResponse(SynonymsReloadResult synonymsReloadResult) { public XContentBuilder toXContent(XContentBuilder builder, Params params) throws IOException { builder.startObject(); { - builder.field("result", updateStatus.name().toLowerCase(Locale.ENGLISH)); + builder.field(RESULT_FIELD, updateStatus.name().toLowerCase(Locale.ENGLISH)); if (reloadAnalyzersResponse != null) { - builder.field("reload_analyzers_details"); + builder.field(RELOAD_ANALYZERS_DETAILS_FIELD); reloadAnalyzersResponse.toXContent(builder, params); } } @@ -66,7 +72,16 @@ public XContentBuilder toXContent(XContentBuilder builder, Params params) throws @Override public void writeTo(StreamOutput out) throws IOException { out.writeEnum(updateStatus); - reloadAnalyzersResponse.writeTo(out); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + out.writeOptionalWriteable(reloadAnalyzersResponse); + } else { + if (reloadAnalyzersResponse == null) { + // Nulls will be written as empty reload analyzer responses for older versions + EMPTY_RELOAD_ANALYZER_RESPONSE.writeTo(out); + } else { + reloadAnalyzersResponse.writeTo(out); + } + } } public RestStatus status() { @@ -76,6 +91,14 @@ public RestStatus status() { }; } + UpdateSynonymsResultStatus updateStatus() { + return updateStatus; + } + + ReloadAnalyzersResponse reloadAnalyzersResponse() { + return reloadAnalyzersResponse; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/SynonymUpdateResponseSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/SynonymUpdateResponseSerializingTests.java index 3f7ca5037e4a7..c1a5b85783e51 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/SynonymUpdateResponseSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/SynonymUpdateResponseSerializingTests.java @@ -9,23 +9,54 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersion; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse; import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponseTests; import org.elasticsearch.common.Strings; import org.elasticsearch.common.io.stream.Writeable; import org.elasticsearch.common.xcontent.XContentHelper; +import org.elasticsearch.synonyms.SynonymsManagementAPIService; import org.elasticsearch.synonyms.SynonymsManagementAPIService.SynonymsReloadResult; -import org.elasticsearch.test.AbstractWireSerializingTestCase; +import org.elasticsearch.test.AbstractBWCSerializationTestCase; +import org.elasticsearch.xcontent.ConstructingObjectParser; +import org.elasticsearch.xcontent.ParseField; +import org.elasticsearch.xcontent.XContentParser; import java.io.IOException; import java.util.Collections; +import java.util.Locale; import java.util.Map; +import static org.elasticsearch.action.synonyms.SynonymUpdateResponse.EMPTY_RELOAD_ANALYZER_RESPONSE; import static org.elasticsearch.synonyms.SynonymsManagementAPIService.UpdateSynonymsResultStatus.CREATED; import static org.elasticsearch.synonyms.SynonymsManagementAPIService.UpdateSynonymsResultStatus.DELETED; import static org.elasticsearch.synonyms.SynonymsManagementAPIService.UpdateSynonymsResultStatus.UPDATED; +import static org.elasticsearch.xcontent.ConstructingObjectParser.constructorArg; +import static org.elasticsearch.xcontent.ConstructingObjectParser.optionalConstructorArg; -public class SynonymUpdateResponseSerializingTests extends AbstractWireSerializingTestCase { +public class SynonymUpdateResponseSerializingTests extends AbstractBWCSerializationTestCase { + + private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + "synonyms_update_response", + true, + arg -> { + SynonymsManagementAPIService.UpdateSynonymsResultStatus status = SynonymsManagementAPIService.UpdateSynonymsResultStatus + .valueOf(((String) arg[0]).toUpperCase(Locale.ROOT)); + ReloadAnalyzersResponse reloadAnalyzersResponse = (ReloadAnalyzersResponse) arg[1]; + return new SynonymUpdateResponse(new SynonymsReloadResult(status, reloadAnalyzersResponse)); + } + ); + + static { + PARSER.declareString(constructorArg(), new ParseField(SynonymUpdateResponse.RESULT_FIELD)); + PARSER.declareObjectOrNull( + optionalConstructorArg(), + (p, c) -> ReloadAnalyzersResponseTests.PARSER.parse(p, null), + null, + new ParseField(SynonymUpdateResponse.RELOAD_ANALYZERS_DETAILS_FIELD) + ); + } @Override protected Writeable.Reader instanceReader() { @@ -34,9 +65,22 @@ protected Writeable.Reader instanceReader() { @Override protected SynonymUpdateResponse createTestInstance() { - Map reloadedIndicesDetails = ReloadAnalyzersResponseTests - .createRandomReloadDetails(); - ReloadAnalyzersResponse reloadAnalyzersResponse = new ReloadAnalyzersResponse(10, 10, 0, null, reloadedIndicesDetails); + return createTestInstance(randomBoolean()); + } + + private SynonymUpdateResponse createTestInstance(boolean includeReloadInfo) { + ReloadAnalyzersResponse reloadAnalyzersResponse = null; + if (includeReloadInfo) { + Map reloadedIndicesDetails = ReloadAnalyzersResponseTests + .createRandomReloadDetails(); + reloadAnalyzersResponse = new ReloadAnalyzersResponse( + randomIntBetween(0, 10), + randomIntBetween(0, 10), + randomIntBetween(0, 5), + null, + reloadedIndicesDetails + ); + } return new SynonymUpdateResponse(new SynonymsReloadResult(randomFrom(CREATED, UPDATED, DELETED), reloadAnalyzersResponse)); } @@ -45,6 +89,17 @@ protected SynonymUpdateResponse mutateInstance(SynonymUpdateResponse instance) t return randomValueOtherThan(instance, this::createTestInstance); } + @Override + protected SynonymUpdateResponse mutateInstanceForVersion(SynonymUpdateResponse instance, TransportVersion version) { + + if (version.before(TransportVersions.SYNONYMS_REFRESH_PARAM) && instance.reloadAnalyzersResponse() == null) { + // Nulls will be written as empty reload analyzer responses for older versions + return new SynonymUpdateResponse(new SynonymsReloadResult(instance.updateStatus(), EMPTY_RELOAD_ANALYZER_RESPONSE)); + } + + return instance; + } + public void testToXContent() throws IOException { Map reloadedIndicesNodes = Collections.singletonMap( "index", @@ -73,4 +128,18 @@ public void testToXContent() throws IOException { } }"""), output); } + + public void testToXContentWithNoReloadResult() throws IOException { + SynonymUpdateResponse response = new SynonymUpdateResponse(new SynonymsReloadResult(CREATED, null)); + String output = Strings.toString(response); + assertEquals(XContentHelper.stripWhitespace(""" + { + "result": "created" + }"""), output); + } + + @Override + protected SynonymUpdateResponse doParseInstance(XContentParser parser) throws IOException { + return PARSER.apply(parser, null); + } } From 0e3f6e79b0d747a94846abd8efd2e9dccd86268b Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 18:51:58 +0200 Subject: [PATCH 27/36] Fix bug in test parser --- .../admin/indices/analyze/ReloadAnalyzersResponseTests.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/ReloadAnalyzersResponseTests.java b/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/ReloadAnalyzersResponseTests.java index 57e56e0907616..036d7273900a3 100644 --- a/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/ReloadAnalyzersResponseTests.java +++ b/server/src/test/java/org/elasticsearch/action/admin/indices/analyze/ReloadAnalyzersResponseTests.java @@ -31,7 +31,7 @@ public class ReloadAnalyzersResponseTests extends AbstractBroadcastResponseTestCase { @SuppressWarnings({ "unchecked" }) - private static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( + public static final ConstructingObjectParser PARSER = new ConstructingObjectParser<>( "reload_analyzer", true, arg -> { @@ -67,8 +67,8 @@ public class ReloadAnalyzersResponseTests extends AbstractBroadcastResponseTestC declareBroadcastFields(PARSER); PARSER.declareObjectArray(constructorArg(), ENTRY_PARSER, ReloadAnalyzersResponse.RELOAD_DETAILS_FIELD); ENTRY_PARSER.declareString(constructorArg(), ReloadAnalyzersResponse.INDEX_FIELD); - ENTRY_PARSER.declareStringArray(constructorArg(), ReloadAnalyzersResponse.RELOADED_ANALYZERS_FIELD); ENTRY_PARSER.declareStringArray(constructorArg(), ReloadAnalyzersResponse.RELOADED_NODE_IDS_FIELD); + ENTRY_PARSER.declareStringArray(constructorArg(), ReloadAnalyzersResponse.RELOADED_ANALYZERS_FIELD); } @Override From 47f8c6ed3d21b24d6118f21ddacc7c8cfdb1981a Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 18:52:10 +0200 Subject: [PATCH 28/36] Spotless --- .../SynonymsManagementAPIServiceIT.java | 97 ++++++++++--------- 1 file changed, 49 insertions(+), 48 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 98564d3a58106..59dd19d8536e9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -138,66 +138,67 @@ public void testCreateTooManySynonymsUsingRuleUpdates() throws InterruptedExcept randomBoolean(), new ActionListener<>() { @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - // Create as many rules as should fail - SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); - CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); - for (int i = 0; i < rulesToUpdate; i++) { - synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], true, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - updatedRulesLatch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); - } - try { - updatedRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { - fail(e); - } - - // Updating more rules fails - int rulesToInsert = rules.length - rulesToUpdate; - CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); - for (int i = rulesToUpdate; i < rulesToInsert; i++) { - synonymsManagementAPIService.putSynonymRule( - // Error here - synonymSetId, - rules[i], - randomBoolean(), - new ActionListener<>() { + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + // Create as many rules as should fail + SynonymRule[] rules = randomSynonymsSet(atLeast(rulesToUpdate + 1)); + CountDownLatch updatedRulesLatch = new CountDownLatch(rulesToUpdate); + for (int i = 0; i < rulesToUpdate; i++) { + synonymsManagementAPIService.putSynonymRule(synonymSetId, rules[i], true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to update a rule"); + updatedRulesLatch.countDown(); } @Override public void onFailure(Exception e) { - if (e instanceof IllegalArgumentException == false) { - fail(e); + fail(e); + } + }); + } + try { + updatedRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } + + // Updating more rules fails + int rulesToInsert = rules.length - rulesToUpdate; + CountDownLatch insertRulesLatch = new CountDownLatch(rulesToInsert); + for (int i = rulesToUpdate; i < rulesToInsert; i++) { + synonymsManagementAPIService.putSynonymRule( + // Error here + synonymSetId, + rules[i], + randomBoolean(), + new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to update a rule"); + } + + @Override + public void onFailure(Exception e) { + if (e instanceof IllegalArgumentException == false) { + fail(e); + } + updatedRulesLatch.countDown(); } - updatedRulesLatch.countDown(); } - } - ); + ); + } + try { + insertRulesLatch.await(5, TimeUnit.SECONDS); + } catch (InterruptedException e) { + fail(e); + } } - try { - insertRulesLatch.await(5, TimeUnit.SECONDS); - } catch (InterruptedException e) { + + @Override + public void onFailure(Exception e) { fail(e); } } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); + ); latch.await(5, TimeUnit.SECONDS); } From 9c8e0b62beaba2f2894756c959cf9df9e20c4b0f Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 19:27:23 +0200 Subject: [PATCH 29/36] Delete doesn't need reloading :facepalm: removing it --- .../api/synonyms.delete_synonym_rule.json | 6 --- .../test/synonyms/70_synonym_rule_delete.yml | 21 ----------- .../SynonymsManagementAPIServiceIT.java | 35 ------------------ .../synonyms/DeleteSynonymRuleAction.java | 18 +-------- .../TransportDeleteSynonymRuleAction.java | 1 - .../synonyms/RestDeleteSynonymRuleAction.java | 3 +- .../SynonymsManagementAPIService.java | 37 +++++++++---------- ...onymRuleActionRequestSerializingTests.java | 2 +- 8 files changed, 22 insertions(+), 101 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json index e2285bbd6d4ae..5a0de4ab94a7c 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json @@ -33,12 +33,6 @@ } } ] - }, - "params": { - "refresh": { - "type": "boolean", - "description": "Refresh search analyzers to update synonyms" - } } } } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml index 50d36b5c5db63..a4853b0b6d414 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml @@ -44,27 +44,6 @@ setup: - synonyms: "test => check" id: "test-id-3" ---- -"Refresh can be specified": - - - requires: - test_runner_features: [ capabilities ] - capabilities: - - method: PUT - path: /_synonyms/{rule_id} - capabilities: [ synonyms_refresh_param ] - reason: "synonyms refresh param capability needed" - - - do: - synonyms.delete_synonym_rule: - set_id: test-synonyms - rule_id: test-id-2 - refresh: false - - - match: { result: "deleted" } - # Reload analyzers info is not included - - not_exists: reload_analyzers_details - --- "Delete synonym rule - missing synonym set": - do: diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 59dd19d8536e9..192a20ed0166d 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -366,25 +366,6 @@ public void onFailure(Exception e) { updateLatch.await(5, TimeUnit.SECONDS); - // Delete a rule fails - CountDownLatch deleteLatch = new CountDownLatch(1); - synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, true, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to delete a synonym rule with refresh in synonyms index health"); - } - - @Override - public void onFailure(Exception e) { - // Expected - assertTrue(e instanceof IndexCreationException); - assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); - updateLatch.countDown(); - } - }); - - deleteLatch.await(5, TimeUnit.SECONDS); - // But, we can still create a synonyms set without refresh CountDownLatch putNoRefreshLatch = new CountDownLatch(1); synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), false, new ActionListener<>() { @@ -418,21 +399,5 @@ public void onFailure(Exception e) { }); putRuleNoRefreshLatch.await(5, TimeUnit.SECONDS); - - // Same for delete - CountDownLatch deleteNoRefreshLatch = new CountDownLatch(1); - synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, false, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - deleteNoRefreshLatch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); - - deleteNoRefreshLatch.await(5, TimeUnit.SECONDS); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java index a485ed19ac742..9f52f1552e421 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java @@ -9,7 +9,6 @@ package org.elasticsearch.action.synonyms; -import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; @@ -32,24 +31,18 @@ public DeleteSynonymRuleAction() { public static class Request extends ActionRequest { private final String synonymsSetId; + private final String synonymRuleId; - private final boolean refresh; public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRuleId = in.readString(); - if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { - this.refresh = in.readBoolean(); - } else { - this.refresh = true; - } } - public Request(String synonymsSetId, String synonymRuleId, boolean refresh) { + public Request(String synonymsSetId, String synonymRuleId) { this.synonymsSetId = synonymsSetId; this.synonymRuleId = synonymRuleId; - this.refresh = refresh; } @Override @@ -70,9 +63,6 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); out.writeString(synonymRuleId); - if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { - out.writeBoolean(refresh); - } } public String synonymsSetId() { @@ -83,10 +73,6 @@ public String synonymRuleId() { return synonymRuleId; } - public boolean refresh() { - return refresh; - } - @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java index 9fed5e5716c5b..0d55774489c4c 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java @@ -41,7 +41,6 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act synonymsManagementAPIService.deleteSynonymRule( request.synonymsSetId(), request.synonymRuleId(), - request.refresh(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java index 9aa8265ce35b6..354f1a73bdffc 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java @@ -40,8 +40,7 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { DeleteSynonymRuleAction.Request request = new DeleteSynonymRuleAction.Request( restRequest.param("synonymsSet"), - restRequest.param("synonymRuleId"), - restRequest.paramAsBoolean("refresh", true) + restRequest.param("synonymRuleId") ); return channel -> client.execute(DeleteSynonymRuleAction.INSTANCE, request, new RestToXContentListener<>(channel)); } diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 8619b11dac0a1..79896822f5883 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -354,7 +354,7 @@ public void putSynonymsSet( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); + checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); }) ); })); @@ -424,7 +424,7 @@ private void indexSynonymRule( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); + checkIndexSearchableAndReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); })); } @@ -444,12 +444,7 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList ); } - public void deleteSynonymRule( - String synonymsSetId, - String synonymRuleId, - boolean refresh, - ActionListener listener - ) { + public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener) { client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId)) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> { @@ -468,7 +463,7 @@ public void deleteSynonymRule( return; } - maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener); + reloadAnalyzers(synonymsSetId, false, UpdateSynonymsResultStatus.DELETED, listener); })); } @@ -525,8 +520,8 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener) { - // Previews reloading the resource to understand its usage on indices - maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { + // Previews reloading the resource to understand its usage on indices. It's OK to reload as we're doing preview mode + reloadAnalyzers(synonymSetId, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -566,7 +561,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void maybeReloadAnalyzers( + private void checkIndexSearchableAndReloadAnalyzers( String synonymSetId, boolean refresh, boolean preview, @@ -596,16 +591,20 @@ private void maybeReloadAnalyzers( return; } - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) - ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); - client.execute( - TransportReloadAnalyzersAction.TYPE, - reloadAnalyzersRequest, - listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) - ); + reloadAnalyzers(synonymSetId, preview, synonymsOperationResult, listener); })); } + private void reloadAnalyzers(String synonymSetId, boolean preview, UpdateSynonymsResultStatus synonymsOperationResult, ActionListener listener) { + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); + client.execute( + TransportReloadAnalyzersAction.TYPE, + reloadAnalyzersRequest, + listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) + ); + } + // Allows checking failures in tests void checkSynonymsIndexHealth(ActionListener listener) { ClusterHealthRequest healthRequest = new ClusterHealthRequest( diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java index 97ca4e22869e1..68adf4034ce51 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java @@ -23,7 +23,7 @@ protected Writeable.Reader instanceReader() { @Override protected DeleteSynonymRuleAction.Request createTestInstance() { - return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier(), randomBoolean()); + return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier()); } @Override From 1792888dc04ad5fe12256d0c8c4990937cc9b741 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 19:33:24 +0200 Subject: [PATCH 30/36] Revert "Delete doesn't need reloading :facepalm: removing it" This reverts commit 9c8e0b62beaba2f2894756c959cf9df9e20c4b0f. --- .../api/synonyms.delete_synonym_rule.json | 6 +++ .../test/synonyms/70_synonym_rule_delete.yml | 21 +++++++++++ .../SynonymsManagementAPIServiceIT.java | 35 ++++++++++++++++++ .../synonyms/DeleteSynonymRuleAction.java | 18 ++++++++- .../TransportDeleteSynonymRuleAction.java | 1 + .../synonyms/RestDeleteSynonymRuleAction.java | 3 +- .../SynonymsManagementAPIService.java | 37 ++++++++++--------- ...onymRuleActionRequestSerializingTests.java | 2 +- 8 files changed, 101 insertions(+), 22 deletions(-) diff --git a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json index 5a0de4ab94a7c..e2285bbd6d4ae 100644 --- a/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json +++ b/rest-api-spec/src/main/resources/rest-api-spec/api/synonyms.delete_synonym_rule.json @@ -33,6 +33,12 @@ } } ] + }, + "params": { + "refresh": { + "type": "boolean", + "description": "Refresh search analyzers to update synonyms" + } } } } diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml index a4853b0b6d414..50d36b5c5db63 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/70_synonym_rule_delete.yml @@ -44,6 +44,27 @@ setup: - synonyms: "test => check" id: "test-id-3" +--- +"Refresh can be specified": + + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: PUT + path: /_synonyms/{rule_id} + capabilities: [ synonyms_refresh_param ] + reason: "synonyms refresh param capability needed" + + - do: + synonyms.delete_synonym_rule: + set_id: test-synonyms + rule_id: test-id-2 + refresh: false + + - match: { result: "deleted" } + # Reload analyzers info is not included + - not_exists: reload_analyzers_details + --- "Delete synonym rule - missing synonym set": - do: diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 192a20ed0166d..59dd19d8536e9 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -366,6 +366,25 @@ public void onFailure(Exception e) { updateLatch.await(5, TimeUnit.SECONDS); + // Delete a rule fails + CountDownLatch deleteLatch = new CountDownLatch(1); + synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, true, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + fail("Shouldn't have been able to delete a synonym rule with refresh in synonyms index health"); + } + + @Override + public void onFailure(Exception e) { + // Expected + assertTrue(e instanceof IndexCreationException); + assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); + updateLatch.countDown(); + } + }); + + deleteLatch.await(5, TimeUnit.SECONDS); + // But, we can still create a synonyms set without refresh CountDownLatch putNoRefreshLatch = new CountDownLatch(1); synonymsManagementAPIService.putSynonymsSet(synonymSetId, randomSynonymsSet(1, 1), false, new ActionListener<>() { @@ -399,5 +418,21 @@ public void onFailure(Exception e) { }); putRuleNoRefreshLatch.await(5, TimeUnit.SECONDS); + + // Same for delete + CountDownLatch deleteNoRefreshLatch = new CountDownLatch(1); + synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, false, new ActionListener<>() { + @Override + public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { + deleteNoRefreshLatch.countDown(); + } + + @Override + public void onFailure(Exception e) { + fail(e); + } + }); + + deleteNoRefreshLatch.await(5, TimeUnit.SECONDS); } } diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java index 9f52f1552e421..a485ed19ac742 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleAction.java @@ -9,6 +9,7 @@ package org.elasticsearch.action.synonyms; +import org.elasticsearch.TransportVersions; import org.elasticsearch.action.ActionRequest; import org.elasticsearch.action.ActionRequestValidationException; import org.elasticsearch.action.ActionType; @@ -31,18 +32,24 @@ public DeleteSynonymRuleAction() { public static class Request extends ActionRequest { private final String synonymsSetId; - private final String synonymRuleId; + private final boolean refresh; public Request(StreamInput in) throws IOException { super(in); this.synonymsSetId = in.readString(); this.synonymRuleId = in.readString(); + if (in.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + this.refresh = in.readBoolean(); + } else { + this.refresh = true; + } } - public Request(String synonymsSetId, String synonymRuleId) { + public Request(String synonymsSetId, String synonymRuleId, boolean refresh) { this.synonymsSetId = synonymsSetId; this.synonymRuleId = synonymRuleId; + this.refresh = refresh; } @Override @@ -63,6 +70,9 @@ public void writeTo(StreamOutput out) throws IOException { super.writeTo(out); out.writeString(synonymsSetId); out.writeString(synonymRuleId); + if (out.getTransportVersion().onOrAfter(TransportVersions.SYNONYMS_REFRESH_PARAM)) { + out.writeBoolean(refresh); + } } public String synonymsSetId() { @@ -73,6 +83,10 @@ public String synonymRuleId() { return synonymRuleId; } + public boolean refresh() { + return refresh; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java index 0d55774489c4c..9fed5e5716c5b 100644 --- a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymRuleAction.java @@ -41,6 +41,7 @@ protected void doExecute(Task task, DeleteSynonymRuleAction.Request request, Act synonymsManagementAPIService.deleteSynonymRule( request.synonymsSetId(), request.synonymRuleId(), + request.refresh(), listener.map(SynonymUpdateResponse::new) ); } diff --git a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java index 354f1a73bdffc..9aa8265ce35b6 100644 --- a/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java +++ b/server/src/main/java/org/elasticsearch/rest/action/synonyms/RestDeleteSynonymRuleAction.java @@ -40,7 +40,8 @@ public List routes() { protected RestChannelConsumer prepareRequest(RestRequest restRequest, NodeClient client) throws IOException { DeleteSynonymRuleAction.Request request = new DeleteSynonymRuleAction.Request( restRequest.param("synonymsSet"), - restRequest.param("synonymRuleId") + restRequest.param("synonymRuleId"), + restRequest.paramAsBoolean("refresh", true) ); return channel -> client.execute(DeleteSynonymRuleAction.INSTANCE, request, new RestToXContentListener<>(channel)); } diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 79896822f5883..8619b11dac0a1 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -354,7 +354,7 @@ public void putSynonymsSet( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); + maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); }) ); })); @@ -424,7 +424,7 @@ private void indexSynonymRule( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - checkIndexSearchableAndReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); + maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); })); } @@ -444,7 +444,12 @@ public void getSynonymRule(String synonymSetId, String synonymRuleId, ActionList ); } - public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, ActionListener listener) { + public void deleteSynonymRule( + String synonymsSetId, + String synonymRuleId, + boolean refresh, + ActionListener listener + ) { client.prepareDelete(SYNONYMS_ALIAS_NAME, internalSynonymRuleId(synonymsSetId, synonymRuleId)) .setRefreshPolicy(WriteRequest.RefreshPolicy.IMMEDIATE) .execute(new DelegatingIndexNotFoundActionListener<>(synonymsSetId, listener, (l, deleteResponse) -> { @@ -463,7 +468,7 @@ public void deleteSynonymRule(String synonymsSetId, String synonymRuleId, Action return; } - reloadAnalyzers(synonymsSetId, false, UpdateSynonymsResultStatus.DELETED, listener); + maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener); })); } @@ -520,8 +525,8 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener) { - // Previews reloading the resource to understand its usage on indices. It's OK to reload as we're doing preview mode - reloadAnalyzers(synonymSetId, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { + // Previews reloading the resource to understand its usage on indices + maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -561,7 +566,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void checkIndexSearchableAndReloadAnalyzers( + private void maybeReloadAnalyzers( String synonymSetId, boolean refresh, boolean preview, @@ -591,20 +596,16 @@ private void checkIndexSearchableAndReloadAnalyzers( return; } - reloadAnalyzers(synonymSetId, preview, synonymsOperationResult, listener); + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); + client.execute( + TransportReloadAnalyzersAction.TYPE, + reloadAnalyzersRequest, + listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) + ); })); } - private void reloadAnalyzers(String synonymSetId, boolean preview, UpdateSynonymsResultStatus synonymsOperationResult, ActionListener listener) { - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) - ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); - client.execute( - TransportReloadAnalyzersAction.TYPE, - reloadAnalyzersRequest, - listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) - ); - } - // Allows checking failures in tests void checkSynonymsIndexHealth(ActionListener listener) { ClusterHealthRequest healthRequest = new ClusterHealthRequest( diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java index 68adf4034ce51..97ca4e22869e1 100644 --- a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java +++ b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymRuleActionRequestSerializingTests.java @@ -23,7 +23,7 @@ protected Writeable.Reader instanceReader() { @Override protected DeleteSynonymRuleAction.Request createTestInstance() { - return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier()); + return new DeleteSynonymRuleAction.Request(randomIdentifier(), randomIdentifier(), randomBoolean()); } @Override From 9b77cbf1d6a864769964bb14cdb74d568d3b8eb0 Mon Sep 17 00:00:00 2001 From: elasticsearchmachine Date: Wed, 16 Apr 2025 17:35:24 +0000 Subject: [PATCH 31/36] [CI] Auto commit changes from spotless --- .../synonyms/SynonymsManagementAPIService.java | 15 +++++++++++++-- 1 file changed, 13 insertions(+), 2 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 79896822f5883..28d93aae102a0 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -354,7 +354,13 @@ public void putSynonymsSet( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); + checkIndexSearchableAndReloadAnalyzers( + synonymSetId, + refresh, + false, + updateSynonymsResultStatus, + bulkInsertResponseListener + ); }) ); })); @@ -595,7 +601,12 @@ private void checkIndexSearchableAndReloadAnalyzers( })); } - private void reloadAnalyzers(String synonymSetId, boolean preview, UpdateSynonymsResultStatus synonymsOperationResult, ActionListener listener) { + private void reloadAnalyzers( + String synonymSetId, + boolean preview, + UpdateSynonymsResultStatus synonymsOperationResult, + ActionListener listener + ) { // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); client.execute( From 098928cb5803d83bb971a6f2669f81ebc2522ffe Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 19:48:50 +0200 Subject: [PATCH 32/36] Fix refresh for delete synonym rule --- .../SynonymsManagementAPIService.java | 32 ++++++++++++------- 1 file changed, 20 insertions(+), 12 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java index 8619b11dac0a1..13c3931f7e7c9 100644 --- a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java +++ b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java @@ -354,7 +354,7 @@ public void putSynonymsSet( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - maybeReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); + checkIndexSearchableAndReloadAnalyzers(synonymSetId, refresh, false, updateSynonymsResultStatus, bulkInsertResponseListener); }) ); })); @@ -424,7 +424,7 @@ private void indexSynonymRule( ? UpdateSynonymsResultStatus.CREATED : UpdateSynonymsResultStatus.UPDATED; - maybeReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); + checkIndexSearchableAndReloadAnalyzers(synonymsSetId, refresh, false, updateStatus, l2); })); } @@ -468,7 +468,11 @@ public void deleteSynonymRule( return; } - maybeReloadAnalyzers(synonymsSetId, refresh, false, UpdateSynonymsResultStatus.DELETED, listener); + if (refresh) { + reloadAnalyzers(synonymsSetId, false, UpdateSynonymsResultStatus.DELETED, listener); + } else { + listener.onResponse(new SynonymsReloadResult(UpdateSynonymsResultStatus.DELETED, null)); + } })); } @@ -526,7 +530,7 @@ private void deleteSynonymsSetObjects(String synonymSetId, ActionListener listener) { // Previews reloading the resource to understand its usage on indices - maybeReloadAnalyzers(synonymSetId, true, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { + reloadAnalyzers(synonymSetId, true, null, listener.delegateFailure((reloadListener, reloadResult) -> { Map reloadDetails = reloadResult.reloadAnalyzersResponse.getReloadDetails(); if (reloadDetails.isEmpty() == false) { Set indices = reloadDetails.entrySet() @@ -566,7 +570,7 @@ public void deleteSynonymsSet(String synonymSetId, ActionListener void maybeReloadAnalyzers( + private void checkIndexSearchableAndReloadAnalyzers( String synonymSetId, boolean refresh, boolean preview, @@ -596,16 +600,20 @@ private void maybeReloadAnalyzers( return; } - // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) - ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); - client.execute( - TransportReloadAnalyzersAction.TYPE, - reloadAnalyzersRequest, - listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) - ); + reloadAnalyzers(synonymSetId, preview, synonymsOperationResult, listener); })); } + private void reloadAnalyzers(String synonymSetId, boolean preview, UpdateSynonymsResultStatus synonymsOperationResult, ActionListener listener) { + // auto-reload all reloadable analyzers (currently only those that use updateable synonym or keyword_marker filters) + ReloadAnalyzersRequest reloadAnalyzersRequest = new ReloadAnalyzersRequest(synonymSetId, preview, "*"); + client.execute( + TransportReloadAnalyzersAction.TYPE, + reloadAnalyzersRequest, + listener.safeMap(reloadResponse -> new SynonymsReloadResult(synonymsOperationResult, reloadResponse)) + ); + } + // Allows checking failures in tests void checkSynonymsIndexHealth(ActionListener listener) { ClusterHealthRequest healthRequest = new ClusterHealthRequest( From 7a426577797f4a15d84b559d4dca874662f56938 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Wed, 16 Apr 2025 20:44:23 +0200 Subject: [PATCH 33/36] Fix tests --- .../SynonymsManagementAPIServiceIT.java | 24 +++---------------- 1 file changed, 3 insertions(+), 21 deletions(-) diff --git a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java index 59dd19d8536e9..9b428321fa6a7 100644 --- a/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java +++ b/server/src/internalClusterTest/java/org/elasticsearch/synonyms/SynonymsManagementAPIServiceIT.java @@ -366,20 +366,18 @@ public void onFailure(Exception e) { updateLatch.await(5, TimeUnit.SECONDS); - // Delete a rule fails + // Delete a rule does not fail CountDownLatch deleteLatch = new CountDownLatch(1); synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, true, new ActionListener<>() { @Override public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - fail("Shouldn't have been able to delete a synonym rule with refresh in synonyms index health"); + updateLatch.countDown(); } @Override public void onFailure(Exception e) { // Expected - assertTrue(e instanceof IndexCreationException); - assertTrue(e.getMessage().contains("synonyms index [.synonyms] is not searchable")); - updateLatch.countDown(); + fail("Should have been able to delete a synonym rule"); } }); @@ -418,21 +416,5 @@ public void onFailure(Exception e) { }); putRuleNoRefreshLatch.await(5, TimeUnit.SECONDS); - - // Same for delete - CountDownLatch deleteNoRefreshLatch = new CountDownLatch(1); - synonymsManagementAPIService.deleteSynonymRule(synonymSetId, synonymRuleId, false, new ActionListener<>() { - @Override - public void onResponse(SynonymsManagementAPIService.SynonymsReloadResult synonymsReloadResult) { - deleteNoRefreshLatch.countDown(); - } - - @Override - public void onFailure(Exception e) { - fail(e); - } - }); - - deleteNoRefreshLatch.await(5, TimeUnit.SECONDS); } } From a0091936f8a84f4026ea06133d515b68f62738d2 Mon Sep 17 00:00:00 2001 From: Carlos Delgado <6339205+carlosdelest@users.noreply.github.com> Date: Thu, 17 Apr 2025 21:20:08 +0200 Subject: [PATCH 34/36] Update docs/changelog/126935.yaml --- docs/changelog/126935.yaml | 6 ++++++ 1 file changed, 6 insertions(+) create mode 100644 docs/changelog/126935.yaml diff --git a/docs/changelog/126935.yaml b/docs/changelog/126935.yaml new file mode 100644 index 0000000000000..7ef231ffa83c4 --- /dev/null +++ b/docs/changelog/126935.yaml @@ -0,0 +1,6 @@ +pr: 126935 +summary: Synonyms API - Add refresh parameter to check synonyms index and reload analyzers +area: Analysis +type: enhancement +issues: + - 121441 From 8c0dca5e409176ee28de409bb83731c8c6caa9c6 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Mon, 21 Apr 2025 10:28:02 +0200 Subject: [PATCH 35/36] Add reload analyzers test --- .../90_synonyms_reloading_for_synset.yml | 57 +++++++++++++++++++ 1 file changed, 57 insertions(+) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml index 579d99e3f2ff7..e0e1f88ef8b03 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml @@ -150,3 +150,60 @@ setup: my_field: query: salute - match: { hits.total.value: 0 } + +--- +"Reload analyzers with refresh false": + - requires: + test_runner_features: [ capabilities ] + capabilities: + - method: PUT + path: /_synonyms/{rule_id} + capabilities: [ synonyms_refresh_param ] + reason: "synonyms refresh param capability needed" + + - do: + synonyms.put_synonym: + id: synonyms_set1 + refresh: false + body: + synonyms_set: + - synonyms: "hello, salute" + + - match: { result: "updated" } + - not_exists: reload_analyzers_details + + # Confirm that the index analyzers are not reloaded for my_index1 + - do: + search: + index: my_index1 + body: + query: + match: + my_field: + query: salute + - match: { hits.total.value: 0 } + + + # This is to ensure that all index shards (write and read) are available. As we won't be using synonyms API to refresh + # analyzers, this can be needed in serverless. + - do: + cluster.health: + index: .synonyms + wait_for_status: green + + # Reloading analyzers makes synonyms refresh + - do: + indices.reload_search_analyzers: + index: my_index1 + - length: { reload_details: 1 } + + - do: + search: + index: my_index1 + body: + query: + match: + my_field: + query: salute + - match: { hits.total.value: 1 } + From fcf5f5a04af81438d10acf5abd3e901b5f462442 Mon Sep 17 00:00:00 2001 From: carlosdelest Date: Tue, 22 Apr 2025 08:56:28 +0200 Subject: [PATCH 36/36] reload_analyzers is not available on serverless --- .../90_synonyms_reloading_for_synset.yml | 24 ++++++++++--------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml index e0e1f88ef8b03..50df0ad2c6327 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml +++ b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/synonyms/90_synonyms_reloading_for_synset.yml @@ -183,19 +183,20 @@ setup: query: salute - match: { hits.total.value: 0 } - - # This is to ensure that all index shards (write and read) are available. As we won't be using synonyms API to refresh - # analyzers, this can be needed in serverless. - - do: - cluster.health: - index: .synonyms - wait_for_status: green - # Reloading analyzers makes synonyms refresh - do: - indices.reload_search_analyzers: - index: my_index1 - - length: { reload_details: 1 } + synonyms.put_synonym: + id: synonyms_set1 + refresh: true + body: + synonyms_set: + - synonyms: "hello, salute" + - synonyms: "ciao => goodbye" + + - match: { result: "updated" } + - gt: { reload_analyzers_details._shards.total: 0 } + - gt: { reload_analyzers_details._shards.successful: 0 } + - length: { reload_analyzers_details.reload_details: 1 } - do: search: @@ -205,5 +206,6 @@ setup: match: my_field: query: salute + - match: { hits.total.value: 1 }