Address feedback 2

elastic · Oct 28, 2020 · 1e427d5 · 1e427d5
1 parent 0775cf1
commit 1e427d5
Show file tree

Hide file tree

Showing 3 changed files with 10 additions and 14 deletions.
diff --git a/docs/reference/search/async-search.asciidoc b/docs/reference/search/async-search.asciidoc
@@ -292,9 +292,9 @@ code of the completed async search.
   "expiration_time_in_millis" : 1584377890986,
   "_shards" : {
       "total" : 562,
-      "successful" : 188,
+      "successful" : 450,
       "skipped" : 0,
-      "failed" : 1
+      "failed" : 112
   },
  "completion_status" : 503 <1>
 }

diff --git a/...ugin/async-search/src/main/java/org/elasticsearch/xpack/search/MutableSearchResponse.java b/...ugin/async-search/src/main/java/org/elasticsearch/xpack/search/MutableSearchResponse.java
@@ -42,7 +42,7 @@ class MutableSearchResponse {
     private final ThreadContext threadContext;
 
     private boolean isPartial;
-    private volatile int successfulShards;
+    private int successfulShards;
     private TotalHits totalHits;
     /**
      * How we get the reduced aggs when {@link #finalResponse} isn't populated.
@@ -56,8 +56,8 @@ class MutableSearchResponse {
      * building our own {@linkplain SearchResponse}s when get async search
      * is called, and instead return this.
      */
-    private volatile SearchResponse finalResponse;
-    private volatile ElasticsearchException failure;
+    private SearchResponse finalResponse;
+    private ElasticsearchException failure;
     private Map<String, List<String>> responseHeaders;
 
     private boolean frozen;
@@ -196,8 +196,8 @@ synchronized AsyncSearchResponse toAsyncSearchResponse(AsyncSearchTask task,
      * @param expirationTime – expiration time of async search request
      * @return response representing the status of async search
      */
-     AsyncStatusResponse toStatusResponse(String asyncExecutionId, long startTime, long expirationTime) {
-        if (finalResponse != null) { // no need to synchronize this, as finalResponse is set only once
+    synchronized AsyncStatusResponse toStatusResponse(String asyncExecutionId, long startTime, long expirationTime) {
+        if (finalResponse != null) {
             return new AsyncStatusResponse(
                 asyncExecutionId,
                 false,
@@ -211,7 +211,7 @@ AsyncStatusResponse toStatusResponse(String asyncExecutionId, long startTime, lo
                 finalResponse.status()
             );
         }
-        if (failure != null) { // no need to synchronize this, as failure is set only once
+        if (failure != null) {
             return new AsyncStatusResponse(
                 asyncExecutionId,
                 false,

diff --git a/...nc-search/src/main/java/org/elasticsearch/xpack/search/TransportGetAsyncStatusAction.java b/...nc-search/src/main/java/org/elasticsearch/xpack/search/TransportGetAsyncStatusAction.java
@@ -70,12 +70,8 @@ private void retrieveStatus(GetAsyncStatusRequest request, ActionListener<AsyncS
         try {
             AsyncTask task = (AsyncTask) taskManager.getTask(searchId.getTaskId().getId());
             if ((task instanceof AsyncSearchTask) && (task.getExecutionId().equals(searchId))) {
-                if (task.isCancelled()) {
-                    listener.onFailure(new ResourceNotFoundException(searchId.getEncoded()));
-                } else {
-                    AsyncStatusResponse response = ((AsyncSearchTask) task).getStatusResponse();
-                    sendFinalResponse(request, response, nowInMillis, listener);
-                }
+                AsyncStatusResponse response = ((AsyncSearchTask) task).getStatusResponse();
+                sendFinalResponse(request, response, nowInMillis, listener);
             } else {
                 getStatusResponseFromIndex(searchId, request, nowInMillis, listener);
             }