Async search: remove version from response (#53960)

The goal of the version field was to quickly show when you can expect to find something new in the search response, compared to when nothing has changed. This can also be done by looking at the `_shards` section and `num_reduce_phases` returned with the search response. In fact when there has been one or more additional reduction of the results, you can expect new results in the search response. Otherwise, the `_shards` section could notify of additional failures of shards that have completed the query, but that is not a guarantee that their results will be exposed (only when the following partial reduction is performed their results will be available).

That said this commit clarifies this in the docs and removes the version field from the async search response

client/rest-high-level/src/main/java/org/elasticsearch/client/asyncsearch/AsyncSearchResponse.java

@@ -41,7 +41,6 @@ import static org.elasticsearch.common.xcontent.XContentParserUtils.ensureExpect
 public class AsyncSearchResponse implements ToXContentObject  {
     @Nullable
     private final String id;
-    private final int version;
     @Nullable
     private final SearchResponse searchResponse;
     @Nullable
@@ -55,15 +54,13 @@ public class AsyncSearchResponse implements ToXContentObject  {
     /**
      * Creates an {@link AsyncSearchResponse} with the arguments that are always present in the server response
      */
-    AsyncSearchResponse(int version,
-                               boolean isPartial,
-                               boolean isRunning,
-                               long startTimeMillis,
-                               long expirationTimeMillis,
-                               @Nullable String id,
-                               @Nullable SearchResponse searchResponse,
-                               @Nullable ElasticsearchException error) {
-        this.version = version;
+    AsyncSearchResponse(boolean isPartial,
+                        boolean isRunning,
+                        long startTimeMillis,
+                        long expirationTimeMillis,
+                        @Nullable String id,
+                        @Nullable SearchResponse searchResponse,
+                        @Nullable ElasticsearchException error) {
         this.isPartial = isPartial;
         this.isRunning = isRunning;
         this.startTimeMillis = startTimeMillis;
@@ -81,13 +78,6 @@ public class AsyncSearchResponse implements ToXContentObject  {
         return id;
     }
 
-    /**
-     * Returns the version of this response.
-     */
-    public int getVersion() {
-        return version;
-    }
-
     /**
      * Returns the current {@link SearchResponse} or <code>null</code> if not available.
      *
@@ -145,7 +135,6 @@ public class AsyncSearchResponse implements ToXContentObject  {
         if (id != null) {
             builder.field("id", id);
         }
-        builder.field("version", version);
         builder.field("is_partial", isPartial);
         builder.field("is_running", isRunning);
         builder.field("start_time_in_millis", startTimeMillis);
@@ -165,7 +154,6 @@ public class AsyncSearchResponse implements ToXContentObject  {
     }
 
     public static final ParseField ID_FIELD = new ParseField("id");
-    public static final ParseField VERSION_FIELD = new ParseField("version");
     public static final ParseField IS_PARTIAL_FIELD = new ParseField("is_partial");
     public static final ParseField IS_RUNNING_FIELD = new ParseField("is_running");
     public static final ParseField START_TIME_FIELD = new ParseField("start_time_in_millis");
@@ -176,16 +164,14 @@ public class AsyncSearchResponse implements ToXContentObject  {
     public static final ConstructingObjectParser<AsyncSearchResponse, Void> PARSER = new ConstructingObjectParser<>(
             "submit_async_search_response", true,
             args -> new AsyncSearchResponse(
-                    (int) args[0],
+                    (boolean) args[0],
                     (boolean) args[1],
-                    (boolean) args[2],
+                    (long) args[2],
                     (long) args[3],
-                    (long) args[4],
-                    (String) args[5],
-                    (SearchResponse) args[6],
-                    (ElasticsearchException) args[7]));
+                    (String) args[4],
+                    (SearchResponse) args[5],
+                    (ElasticsearchException) args[6]));
     static {
-        PARSER.declareInt(constructorArg(), VERSION_FIELD);
         PARSER.declareBoolean(constructorArg(), IS_PARTIAL_FIELD);
         PARSER.declareBoolean(constructorArg(), IS_RUNNING_FIELD);
         PARSER.declareLong(constructorArg(), START_TIME_FIELD);
@@ -203,7 +189,7 @@ public class AsyncSearchResponse implements ToXContentObject  {
         return SearchResponse.innerFromXContent(p);
     }
 
-    public static AsyncSearchResponse fromXContent(XContentParser parser) throws IOException {
+    public static AsyncSearchResponse fromXContent(XContentParser parser) {
         return PARSER.apply(parser, null);
     }
 

client/rest-high-level/src/test/java/org/elasticsearch/client/asyncsearch/AsyncSearchIT.java

@@ -40,7 +40,6 @@ public class AsyncSearchIT extends ESRestHighLevelClientTestCase {
         // 15 sec should be enough to make sure we always complete right away
         request.setWaitForCompletion(new TimeValue(15, TimeUnit.SECONDS));
         AsyncSearchResponse response = highLevelClient().asyncSearch().submitAsyncSearch(request, RequestOptions.DEFAULT);
-        assertTrue(response.getVersion() >= 0);
         assertFalse(response.isPartial());
         assertTrue(response.getStartTime() > 0);
         assertTrue(response.getExpirationTime() > 0);

client/rest-high-level/src/test/java/org/elasticsearch/client/asyncsearch/AsyncSearchResponseTests.java

@@ -36,7 +36,6 @@ public class AsyncSearchResponseTests
 
     @Override
     protected org.elasticsearch.xpack.core.search.action.AsyncSearchResponse createServerTestInstance(XContentType xContentType) {
-        int version = randomIntBetween(0, Integer.MAX_VALUE);
         boolean isPartial = randomBoolean();
         boolean isRunning = randomBoolean();
         long startTimeMillis = randomLongBetween(0, Long.MAX_VALUE);
@@ -48,7 +47,7 @@ public class AsyncSearchResponseTests
                 : new SearchResponse(InternalSearchResponse.empty(), randomAlphaOfLength(10), 1, 1, 0, randomIntBetween(0, 10000),
                         ShardSearchFailure.EMPTY_ARRAY, Clusters.EMPTY);
         org.elasticsearch.xpack.core.search.action.AsyncSearchResponse testResponse =
-                new org.elasticsearch.xpack.core.search.action.AsyncSearchResponse(id, version, searchResponse, error, isPartial, isRunning,
+                new org.elasticsearch.xpack.core.search.action.AsyncSearchResponse(id, searchResponse, error, isPartial, isRunning,
                         startTimeMillis, expirationTimeMillis);
         return testResponse;
     }
@@ -62,7 +61,6 @@ public class AsyncSearchResponseTests
     protected void assertInstances(org.elasticsearch.xpack.core.search.action.AsyncSearchResponse expected, AsyncSearchResponse parsed) {
         assertNotSame(parsed, expected);
         assertEquals(expected.getId(), parsed.getId());
-        assertEquals(expected.getVersion(), parsed.getVersion());
         assertEquals(expected.isRunning(), parsed.isRunning());
         assertEquals(expected.isPartial(), parsed.isPartial());
         assertEquals(expected.getStartTime(), parsed.getStartTime());

docs/reference/search/async-search.asciidoc

@@ -43,7 +43,6 @@ results are returned as part of the <<search-api-response-body,`response`>> obje
 --------------------------------------------------
 {
   "id" : "FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsdzoxMDc=", <1>
-  "version" : 0,
   "is_partial" : true, <2>
   "is_running" : true, <3>
   "start_time_in_millis" : 1583945890986,
@@ -135,17 +134,16 @@ GET /_async_search/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsd
 --------------------------------------------------
 {
   "id" : "FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsdzoxMDc=",
-  "version" : 2, <1>
-  "is_partial" : true, <2>
-  "is_running" : true, <3>
+  "is_partial" : true, <1>
+  "is_running" : true, <2>
   "start_time_in_millis" : 1583945890986,
-  "expiration_time_in_millis" : 1584377890986, <4>
+  "expiration_time_in_millis" : 1584377890986, <3>
   "response" : {
     "took" : 12144,
     "timed_out" : false,
-    "num_reduce_phases" : 38,
+    "num_reduce_phases" : 38, <4>
     "_shards" : {
-      "total" : 562,
+      "total" : 562, <5>
       "successful" : 188,
       "skipped" : 0,
       "failed" : 0
@@ -158,7 +156,7 @@ GET /_async_search/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsd
       "max_score" : null,
       "hits" : [ ]
     },
-    "aggregations" : { <5>
+    "aggregations" : { <6>
       "sale_date" :  {
         "buckets" : []
       }
@@ -178,15 +176,16 @@ GET /_async_search/FmRldE8zREVEUzA2ZVpUeGs2ejJFUFEaMkZ5QTVrSTZSaVN3WlNFVmtlWHJsd
 // TESTRESPONSE[s/"buckets" : \[\]/"buckets": $body.response.aggregations.sale_date.buckets/]
 // TESTRESPONSE[s/"num_reduce_phases" : 38,//]
 
-<1> The returned `version` is useful to identify whether the response contains
-additional results compared to previously obtained responses. If the version
-stays the same, no new results have become available, otherwise a higher version
-number indicates that more shards have completed their execution of the query
-and their partial results are also included in the response.
-<2> Whether the returned search results are partial or final
-<3> Whether the search is still being executed or it has completed
-<4> When the async search will expire
-<5> Partial aggregations results, coming from the shards that have already
+<1> Whether the returned search results are partial or final
+<2> Whether the search is still being executed or it has completed
+<3> When the async search will expire
+<4> Indicates how many reduction of the results have been performed. If this
+number increases compared to the last retrieved results, you can expect
+additional results included in the search response
+<5> Indicates how many shards have executed the query. Note that in order for
+shard results to be included in the search response, they need to be reduced
+first.
+<6> Partial aggregations results, coming from the shards that have already
 completed the execution of the query.
 
 The `wait_for_completion` parameter, which defaults to `1`, can also be provided

x-pack/plugin/async-search/src/main/java/org/elasticsearch/xpack/search/MutableSearchResponse.java

@@ -40,7 +40,6 @@ class MutableSearchResponse {
     private final AtomicArray<ShardSearchFailure> shardFailures;
     private final Supplier<InternalAggregation.ReduceContext> aggReduceContextSupplier;
 
-    private int version;
     private boolean isPartial;
     private boolean isFinalReduce;
     private int successfulShards;
@@ -63,7 +62,6 @@ class MutableSearchResponse {
         this.skippedShards = skippedShards;
         this.clusters = clusters;
         this.aggReduceContextSupplier = aggReduceContextSupplier;
-        this.version = 0;
         this.shardFailures = totalShards == -1 ? null : new AtomicArray<>(totalShards-skippedShards);
         this.isPartial = true;
         this.sections = totalShards == -1 ? null : new InternalSearchResponse(
@@ -83,7 +81,6 @@ class MutableSearchResponse {
             throw new IllegalStateException("received partial response out of order: "
                 + newSections.getNumReducePhases() + " < " + sections.getNumReducePhases());
         }
-        ++ version;
         this.successfulShards = successfulShards;
         this.sections = newSections;
         this.isPartial = true;
@@ -96,7 +93,6 @@ class MutableSearchResponse {
      */
     synchronized void updateFinalResponse(int successfulShards, SearchResponseSections newSections) {
         failIfFrozen();
-        ++ version;
         this.successfulShards = successfulShards;
         this.sections = newSections;
         this.isPartial = false;
@@ -110,7 +106,6 @@ class MutableSearchResponse {
      */
     synchronized void updateWithFailure(Exception exc) {
         failIfFrozen();
-        ++ version;
         this.isPartial = true;
         this.failure = ElasticsearchException.guessRootCauses(exc)[0];
         this.frozen = true;
@@ -147,7 +142,7 @@ class MutableSearchResponse {
         } else {
             resp = null;
         }
-        return new AsyncSearchResponse(task.getSearchId().getEncoded(), version, resp, failure, isPartial,
+        return new AsyncSearchResponse(task.getSearchId().getEncoded(), resp, failure, isPartial,
             frozen == false, task.getStartTime(), expirationTime);
     }
 
@@ -159,7 +154,7 @@ class MutableSearchResponse {
 
     private ShardSearchFailure[] buildShardFailures() {
         if (shardFailures == null) {
-            return new ShardSearchFailure[0];
+            return ShardSearchFailure.EMPTY_ARRAY;
         }
         List<ShardSearchFailure> failures = new ArrayList<>();
         for (int i = 0; i < shardFailures.length(); i++) {

x-pack/plugin/async-search/src/test/java/org/elasticsearch/xpack/search/AsyncSearchResponseTests.java

@@ -91,15 +91,15 @@ public class AsyncSearchResponseTests extends ESTestCase {
         int rand = randomIntBetween(0, 2);
         switch (rand) {
             case 0:
-                return new AsyncSearchResponse(searchId, randomIntBetween(0, Integer.MAX_VALUE), randomBoolean(),
+                return new AsyncSearchResponse(searchId, randomBoolean(),
                     randomBoolean(), randomNonNegativeLong(), randomNonNegativeLong());
 
             case 1:
-                return new AsyncSearchResponse(searchId, randomIntBetween(0, Integer.MAX_VALUE), searchResponse, null,
+                return new AsyncSearchResponse(searchId, searchResponse, null,
                     randomBoolean(), randomBoolean(), randomNonNegativeLong(), randomNonNegativeLong());
 
             case 2:
-                return new AsyncSearchResponse(searchId, randomIntBetween(0, Integer.MAX_VALUE), searchResponse,
+                return new AsyncSearchResponse(searchId, searchResponse,
                     new ElasticsearchException(new IOException("boum")), randomBoolean(), randomBoolean(),
                     randomNonNegativeLong(), randomNonNegativeLong());
 
@@ -120,7 +120,6 @@ public class AsyncSearchResponseTests extends ESTestCase {
 
     static void assertEqualResponses(AsyncSearchResponse expected, AsyncSearchResponse actual) {
         assertEquals(expected.getId(), actual.getId());
-        assertEquals(expected.getVersion(), actual.getVersion());
         assertEquals(expected.status(), actual.status());
         assertEquals(expected.getFailure() == null, actual.getFailure() == null);
         assertEquals(expected.isRunning(), actual.isRunning());

x-pack/plugin/core/src/main/java/org/elasticsearch/xpack/core/search/action/AsyncSearchResponse.java

@@ -25,7 +25,6 @@ import static org.elasticsearch.rest.RestStatus.OK;
 public class AsyncSearchResponse extends ActionResponse implements StatusToXContentObject {
     @Nullable
     private final String id;
-    private final int version;
     @Nullable
     private final SearchResponse searchResponse;
     @Nullable
@@ -40,19 +39,17 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
      * Creates an {@link AsyncSearchResponse} with meta-information only (not-modified).
      */
     public AsyncSearchResponse(String id,
-                               int version,
                                boolean isPartial,
                                boolean isRunning,
                                long startTimeMillis,
                                long expirationTimeMillis) {
-        this(id, version, null, null, isPartial, isRunning, startTimeMillis, expirationTimeMillis);
+        this(id, null, null, isPartial, isRunning, startTimeMillis, expirationTimeMillis);
     }
 
     /**
      * Creates a new {@link AsyncSearchResponse}
      *
      * @param id The id of the search for further retrieval, <code>null</code> if not stored.
-     * @param version The version number of this response.
      * @param searchResponse The actual search response.
      * @param error The error if the search failed, <code>null</code> if the search is running
      *                or has completed without failure.
@@ -61,7 +58,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
      * @param startTimeMillis The start date of the search in milliseconds since epoch.
      */
     public AsyncSearchResponse(String id,
-                               int version,
                                SearchResponse searchResponse,
                                ElasticsearchException error,
                                boolean isPartial,
@@ -69,7 +65,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
                                long startTimeMillis,
                                long expirationTimeMillis) {
         this.id = id;
-        this.version = version;
         this.error = error;
         this.searchResponse = searchResponse;
         this.isPartial = isPartial;
@@ -80,7 +75,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
 
     public AsyncSearchResponse(StreamInput in) throws IOException {
         this.id = in.readOptionalString();
-        this.version = in.readVInt();
         this.error = in.readOptionalWriteable(ElasticsearchException::new);
         this.searchResponse = in.readOptionalWriteable(SearchResponse::new);
         this.isPartial = in.readBoolean();
@@ -92,7 +86,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
     @Override
     public void writeTo(StreamOutput out) throws IOException {
         out.writeOptionalString(id);
-        out.writeVInt(version);
         out.writeOptionalWriteable(error);
         out.writeOptionalWriteable(searchResponse);
         out.writeBoolean(isPartial);
@@ -102,7 +95,7 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
     }
 
     public AsyncSearchResponse clone(String id) {
-        return new AsyncSearchResponse(id, version, searchResponse, error, isPartial, false, startTimeMillis, expirationTimeMillis);
+        return new AsyncSearchResponse(id, searchResponse, error, isPartial, false, startTimeMillis, expirationTimeMillis);
     }
 
     /**
@@ -113,13 +106,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
         return id;
     }
 
-    /**
-     * Returns the version of this response.
-     */
-    public int getVersion() {
-        return version;
-    }
-
     /**
      * Returns the current {@link SearchResponse} or <code>null</code> if not available.
      *
@@ -189,7 +175,6 @@ public class AsyncSearchResponse extends ActionResponse implements StatusToXCont
         if (id != null) {
             builder.field("id", id);
         }
-        builder.field("version", version);
         builder.field("is_partial", isPartial);
         builder.field("is_running", isRunning);
         builder.field("start_time_in_millis", startTimeMillis);

x-pack/plugin/src/test/resources/rest-api-spec/test/async_search/10_basic.yml

@@ -54,7 +54,6 @@
           sort: max
 
   - is_false: id
-  - match:  { version:                          6 }
   - match:  { is_partial:                   false }
   - length: { response.hits.hits:               3 }
   - match:  { response.hits.hits.0._source.max: 1 }
@@ -73,7 +72,6 @@
                 field: max
           sort: max
 
-  - match:  { version:                          6 }
   - match:  { is_partial:                   false }
   - length: { response.hits.hits:               3 }
   - match:  { response.hits.hits.0._source.max: 1 }
@@ -95,7 +93,6 @@
           sort: max
 
   - set:    { id:                                  id }
-  - match:  { version:                              6 }
   - match:  { is_partial:                       false }
   - is_false: response._clusters
   - length: { response.hits.hits:                   3 }
@@ -106,7 +103,6 @@
       async_search.get:
         id: "$id"
 
-  - match:  { version:                            6 }
   - match:  { is_partial:                     false }
   - is_false: response._clusters
   - length: { response.hits.hits:                 3 }
@@ -119,7 +115,6 @@
         id: "$id"
         typed_keys: true
 
-  - match:  { version:                              6 }
   - match:  { is_partial:                       false }
   - is_false: response._clusters
   - length: { response.hits.hits:                   3 }