Auto-reload synonym analyzers on synonyms updates (#96886)

mayya-sharipova · web-flow · commit b508ee7886d1 · 2023-06-19T10:50:02.000-04:00
Synonym Management API project

On changes of synonyms in a synonym set, auto-reload analyzers.
Note that currently all updateable analyzers will be reloaded, even
those that are not relevant for a synonyms set being updated.
diff --git a/modules/analysis-common/build.gradle b/modules/analysis-common/build.gradle
@@ -19,7 +19,7 @@ esplugin {
 
 restResources {
   restApi {
-    include '_common', 'indices', 'index', 'cluster', 'search', 'nodes', 'bulk', 'termvectors', 'explain', 'count', 'synonyms.put'
+    include '_common', 'indices', 'index', 'cluster', 'search', 'nodes', 'bulk', 'termvectors', 'explain', 'count', 'synonyms.put', 'synonyms.delete'
   }
 }
 
diff --git a/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/analysis-common/70_synonyms_from_index.yml b/modules/analysis-common/src/yamlRestTest/resources/rest-api-spec/test/analysis-common/70_synonyms_from_index.yml
@@ -1,18 +1,20 @@
 ---
-"Load synonyms from index for an analyzer":
+"Load and auto-reload synonyms from index for an analyzer":
   - skip:
       version: " - 8.8.99"
       reason: Loading synonyms from index is introduced in 8.9.0
+
+  # Create a new synonyms set
   - do:
       synonyms.put:
         synonyms_set: set1
         body:
           synonyms_set:
             - synonyms: "hello, hi"
             - synonyms: "bye => goodbye"
-
   - match: { result: "created" }
 
+  # Create an index with synonym_filter that uses that synonyms set
   - do:
       indices.create:
         index: my_index
@@ -46,6 +48,7 @@
           - '{"index": {"_index": "my_index", "_id": "2"}}'
           - '{"my_field": "goodbye"}'
 
+  # Confirm that synonyms from the synonyms set are used
   - do:
       search:
         index: my_index
@@ -66,6 +69,71 @@
                 query: bye
   - match: { hits.total.value: 1 }
 
+  # Update synonyms and check reload status
+  - do:
+      synonyms.put:
+        synonyms_set: set1
+        body:
+          synonyms_set:
+            - synonyms: "hello, salute"
+            - synonyms: "ciao => goodbye"
+  - match: { result: "updated" }
+  - gte: { reload_analyzers_details._shards.total: 1 }
+  - match: { reload_analyzers_details.reload_details.0.index: "my_index" }
+  - match: { reload_analyzers_details.reload_details.0.reloaded_analyzers.0 : "my_analyzer" }
+
+  # Confirm that the index analyzers are reloaded
+  - do:
+      search:
+        index: my_index
+        body:
+          query:
+            match:
+              my_field:
+                query: salute
+  - match: { hits.total.value: 1 }
+
+  - do:
+      search:
+        index: my_index
+        body:
+          query:
+            match:
+              my_field:
+                query: ciao
+  - match: { hits.total.value: 1 }
+
+  # Delete the synonyms set and confirm failed reload analyzers details
+  - do:
+      synonyms.delete:
+        synonyms_set: set1
+
+  - match:
+      acknowledged: true
+  - gte: { reload_analyzers_details._shards.failed: 1 }
+  - match: { reload_analyzers_details._shards.failures.0.index: "my_index" }
+  - match: { reload_analyzers_details._shards.failures.0.reason.reason: "Synonym set [set1] not found" }
+
+  # Confirm that the index analyzers are not reloaded and still using old synonyms
+  - do:
+      search:
+        index: my_index
+        body:
+          query:
+            match:
+              my_field:
+                query: salute
+  - match: { hits.total.value: 1 }
+
+  - do:
+      search:
+        index: my_index
+        body:
+          query:
+            match:
+              my_field:
+                query: ciao
+  - match: { hits.total.value: 1 }
 
 ---
 "Fail loading synonyms from index if synonyms_set doesn't exist":
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
@@ -13,6 +13,8 @@
               id: "test-id"
 
   - match: { result: "created" }
+  - match: { reload_analyzers_details._shards.total: 0 }
+  - length: { reload_analyzers_details.reload_details: 0 }
 
   - do:
       synonyms.put:
@@ -22,6 +24,8 @@
             - synonyms: "other, another"
 
   - match: { result: "updated" }
+  - match: { reload_analyzers_details._shards.total: 0 }
+  - length: { reload_analyzers_details.reload_details: 0 }
 
 ---
 "Validation fails tests":
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
@@ -20,6 +20,8 @@ setup:
 
   - match:
       acknowledged: true
+  - match: { reload_analyzers_details._shards.total: 0 }
+  - length: { reload_analyzers_details.reload_details: 0 }
 
   - do:
       catch: missing
diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/DeleteSynonymsAction.java
@@ -11,21 +11,28 @@
 import org.apache.logging.log4j.util.Strings;
 import org.elasticsearch.action.ActionRequest;
 import org.elasticsearch.action.ActionRequestValidationException;
+import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.ActionType;
+import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse;
 import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
+import org.elasticsearch.xcontent.ToXContent;
+import org.elasticsearch.xcontent.ToXContentObject;
+import org.elasticsearch.xcontent.XContentBuilder;
 
 import java.io.IOException;
 import java.util.Objects;
 
-public class DeleteSynonymsAction extends ActionType<AcknowledgedResponse> {
+import static org.elasticsearch.action.support.master.AcknowledgedResponse.ACKNOWLEDGED_KEY;
+
+public class DeleteSynonymsAction extends ActionType<DeleteSynonymsAction.Response> {
 
     public static final DeleteSynonymsAction INSTANCE = new DeleteSynonymsAction();
     public static final String NAME = "cluster:admin/synonyms/delete";
 
     public DeleteSynonymsAction() {
-        super(NAME, AcknowledgedResponse::readFrom);
+        super(NAME, Response::new);
     }
 
     public static class Request extends ActionRequest {
@@ -71,4 +78,56 @@ public int hashCode() {
             return Objects.hash(synonymsSetId);
         }
     }
+
+    public static class Response extends ActionResponse implements ToXContentObject {
+
+        private final AcknowledgedResponse acknowledgedResponse;
+        private final ReloadAnalyzersResponse reloadAnalyzersResponse;
+
+        public Response(StreamInput in) throws IOException {
+            super(in);
+            this.acknowledgedResponse = AcknowledgedResponse.readFrom(in);
+            this.reloadAnalyzersResponse = new ReloadAnalyzersResponse(in);
+        }
+
+        public Response(AcknowledgedResponse acknowledgedResponse, ReloadAnalyzersResponse reloadAnalyzersResponse) {
+            super();
+            Objects.requireNonNull(acknowledgedResponse, "Acknowledge response must not be null");
+            Objects.requireNonNull(reloadAnalyzersResponse, "Reload analyzers response must not be null");
+            this.acknowledgedResponse = acknowledgedResponse;
+            this.reloadAnalyzersResponse = reloadAnalyzersResponse;
+        }
+
+        @Override
+        public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
+            builder.startObject();
+            {
+                builder.field(ACKNOWLEDGED_KEY, acknowledgedResponse.isAcknowledged());
+                builder.field("reload_analyzers_details");
+                reloadAnalyzersResponse.toXContent(builder, params);
+            }
+            builder.endObject();
+            return builder;
+        }
+
+        @Override
+        public void writeTo(StreamOutput out) throws IOException {
+            acknowledgedResponse.writeTo(out);
+            reloadAnalyzersResponse.writeTo(out);
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (this == o) return true;
+            if (o == null || getClass() != o.getClass()) return false;
+            Response response = (Response) o;
+            return Objects.equals(acknowledgedResponse, response.acknowledgedResponse)
+                && Objects.equals(reloadAnalyzersResponse, response.reloadAnalyzersResponse);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(acknowledgedResponse, reloadAnalyzersResponse);
+        }
+    }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/PutSynonymsAction.java
@@ -13,6 +13,7 @@
 import org.elasticsearch.action.ActionResponse;
 import org.elasticsearch.action.ActionType;
 import org.elasticsearch.action.ValidateActions;
+import org.elasticsearch.action.admin.indices.analyze.ReloadAnalyzersResponse;
 import org.elasticsearch.common.Strings;
 import org.elasticsearch.common.bytes.BytesReference;
 import org.elasticsearch.common.io.stream.StreamInput;
@@ -22,6 +23,7 @@
 import org.elasticsearch.rest.RestStatus;
 import org.elasticsearch.synonyms.SynonymRule;
 import org.elasticsearch.synonyms.SynonymsManagementAPIService;
+import org.elasticsearch.synonyms.SynonymsManagementAPIService.UpdateSynonymsResultStatus;
 import org.elasticsearch.xcontent.ConstructingObjectParser;
 import org.elasticsearch.xcontent.ParseField;
 import org.elasticsearch.xcontent.ToXContent;
@@ -121,36 +123,42 @@ public int hashCode() {
 
     public static class Response extends ActionResponse implements StatusToXContentObject {
 
-        private final SynonymsManagementAPIService.UpdateSynonymsResult result;
+        private final UpdateSynonymsResultStatus updateStatus;
+        private final ReloadAnalyzersResponse reloadAnalyzersResponse;
 
         public Response(StreamInput in) throws IOException {
             super(in);
-            this.result = in.readEnum((SynonymsManagementAPIService.UpdateSynonymsResult.class));
+            this.updateStatus = in.readEnum(UpdateSynonymsResultStatus.class);
+            this.reloadAnalyzersResponse = new ReloadAnalyzersResponse(in);
         }
 
-        public Response(SynonymsManagementAPIService.UpdateSynonymsResult result) {
+        public Response(UpdateSynonymsResultStatus updateStatus, ReloadAnalyzersResponse reloadAnalyzersResponse) {
             super();
-            Objects.requireNonNull(result, "Result must not be null");
-            this.result = result;
+            Objects.requireNonNull(updateStatus, "Update status must not be null");
+            Objects.requireNonNull(updateStatus, "Reload analyzers response must not be null");
+            this.updateStatus = updateStatus;
+            this.reloadAnalyzersResponse = reloadAnalyzersResponse;
         }
 
         @Override
         public XContentBuilder toXContent(XContentBuilder builder, ToXContent.Params params) throws IOException {
             builder.startObject();
-            builder.field("result", result.name().toLowerCase(Locale.ENGLISH));
+            builder.field("result", updateStatus.name().toLowerCase(Locale.ENGLISH));
+            builder.field("reload_analyzers_details");
+            reloadAnalyzersResponse.toXContent(builder, params);
             builder.endObject();
-
             return builder;
         }
 
         @Override
         public void writeTo(StreamOutput out) throws IOException {
-            out.writeEnum(result);
+            out.writeEnum(updateStatus);
+            reloadAnalyzersResponse.writeTo(out);
         }
 
         @Override
         public RestStatus status() {
-            return switch (result) {
+            return switch (updateStatus) {
                 case CREATED -> RestStatus.CREATED;
                 default -> RestStatus.OK;
             };
@@ -161,12 +169,12 @@ public boolean equals(Object o) {
             if (this == o) return true;
             if (o == null || getClass() != o.getClass()) return false;
             Response response = (Response) o;
-            return result == response.result;
+            return updateStatus == response.updateStatus && Objects.equals(reloadAnalyzersResponse, response.reloadAnalyzersResponse);
         }
 
         @Override
         public int hashCode() {
-            return Objects.hash(result);
+            return Objects.hash(updateStatus, reloadAnalyzersResponse);
         }
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportDeleteSynonymsAction.java
@@ -11,14 +11,13 @@
 import org.elasticsearch.action.ActionListener;
 import org.elasticsearch.action.support.ActionFilters;
 import org.elasticsearch.action.support.HandledTransportAction;
-import org.elasticsearch.action.support.master.AcknowledgedResponse;
 import org.elasticsearch.client.internal.Client;
 import org.elasticsearch.common.inject.Inject;
 import org.elasticsearch.synonyms.SynonymsManagementAPIService;
 import org.elasticsearch.tasks.Task;
 import org.elasticsearch.transport.TransportService;
 
-public class TransportDeleteSynonymsAction extends HandledTransportAction<DeleteSynonymsAction.Request, AcknowledgedResponse> {
+public class TransportDeleteSynonymsAction extends HandledTransportAction<DeleteSynonymsAction.Request, DeleteSynonymsAction.Response> {
 
     private final SynonymsManagementAPIService synonymsManagementAPIService;
 
@@ -30,7 +29,10 @@ public TransportDeleteSynonymsAction(TransportService transportService, ActionFi
     }
 
     @Override
-    protected void doExecute(Task task, DeleteSynonymsAction.Request request, ActionListener<AcknowledgedResponse> listener) {
-        synonymsManagementAPIService.deleteSynonymsSet(request.synonymsSetId(), listener);
+    protected void doExecute(Task task, DeleteSynonymsAction.Request request, ActionListener<DeleteSynonymsAction.Response> listener) {
+        synonymsManagementAPIService.deleteSynonymsSet(
+            request.synonymsSetId(),
+            listener.map(dr -> new DeleteSynonymsAction.Response(dr.acknowledgedResponse(), dr.reloadAnalyzersResponse()))
+        );
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java b/server/src/main/java/org/elasticsearch/action/synonyms/TransportPutSynonymsAction.java
@@ -33,7 +33,7 @@ protected void doExecute(Task task, PutSynonymsAction.Request request, ActionLis
         synonymsManagementAPIService.putSynonymsSet(
             request.synonymsSetId(),
             request.synonymRules(),
-            listener.map(PutSynonymsAction.Response::new)
+            listener.map(ur -> new PutSynonymsAction.Response(ur.updateStatus(), ur.reloadAnalyzersResponse()))
         );
     }
 }
diff --git a/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java b/server/src/main/java/org/elasticsearch/synonyms/SynonymsManagementAPIService.java
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
diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymsActionResponseSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/DeleteSynonymsActionResponseSerializingTests.java
diff --git a/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionResponseSerializingTests.java b/server/src/test/java/org/elasticsearch/action/synonyms/PutSynonymsActionResponseSerializingTests.java

Original file line number	Diff line number	Diff line change
`@@ -19,7 +19,7 @@ esplugin {`
`19`	`19`
`20`	`20`	`restResources {`
`21`	`21`	`restApi {`
`22`		`- include '_common', 'indices', 'index', 'cluster', 'search', 'nodes', 'bulk', 'termvectors', 'explain', 'count', 'synonyms.put'`
	`22`	`+ include '_common', 'indices', 'index', 'cluster', 'search', 'nodes', 'bulk', 'termvectors', 'explain', 'count', 'synonyms.put', 'synonyms.delete'`
`23`	`23`	`}`
`24`	`24`	`}`
`25`	`25`
Original file line number	Diff line number	Diff line change
`@@ -33,7 +33,7 @@ protected void doExecute(Task task, PutSynonymsAction.Request request, ActionLis`
`33`	`33`	`synonymsManagementAPIService.putSynonymsSet(`
`34`	`34`	`request.synonymsSetId(),`
`35`	`35`	`request.synonymRules(),`
`36`		`- listener.map(PutSynonymsAction.Response::new)`
	`36`	`+ listener.map(ur -> new PutSynonymsAction.Response(ur.updateStatus(), ur.reloadAnalyzersResponse()))`
`37`	`37`	`);`
`38`	`38`	`}`
`39`	`39`	`}`