Bläddra i källkod

Only validate afterKey in reduce (#95460)

#95037 added validation of an afterKey to the standard InternalComposite
constructor. However, this could lead to odd circumstances where an afterKey
parse failure only happened on a single shard in a multiply-sharded query,
meaning that the error would appear as a failed shard rather than failing the
whole request.

This commit moves validation outside the constructor entirely and instead
does it in the reduce() call explicitly, so that it always happens on the
co-ordinating node. This means that a) the whole request will be failed if
the afterKey doesn't parse, and b) we only fail if the afterKey that is actually
sent back to the user doesn't parse.
Alan Woodward 2 år sedan
förälder
incheckning
0e0dc445dd

+ 2 - 4
modules/aggregations/src/yamlRestTest/resources/rest-api-spec/test/aggregations/composite.yml

@@ -620,10 +620,8 @@ setup:
 ---
 "Composite aggregation with lossy format":
   - skip:
-      # version: " - 7.13.99"
-      # reason:  After key parse checking added in 7.14
-      version: "all"
-      reason: "AwaitsFix https://github.com/elastic/elasticsearch/issues/95386"
+      version: " - 7.13.99"
+      reason:  After key parse checking added in 7.14
 
   - do:
       catch: /created output it couldn't parse/

+ 4 - 3
server/src/main/java/org/elasticsearch/search/aggregations/bucket/composite/InternalComposite.java

@@ -69,13 +69,12 @@ public class InternalComposite extends InternalMultiBucketAggregation<InternalCo
         this.reverseMuls = reverseMuls;
         this.missingOrders = missingOrders;
         this.earlyTerminated = earlyTerminated;
-        validateAfterKey();
     }
 
     /**
      * Checks that the afterKey formatting does not result in loss of information
      *
-     * Only called when a new InternalComposite() is built directly.  We can't validate afterKeys from
+     * Only called when a new InternalComposite() is built after a reduce.  We can't validate afterKeys from
      * InternalComposites built from a StreamInput because they may be coming from nodes that do not
      * do validation, and errors thrown during StreamInput deserialization can kill a node.  However,
      * InternalComposites that come from remote nodes will always be reduced on the co-ordinator, and
@@ -261,7 +260,7 @@ public class InternalComposite extends InternalMultiBucketAggregation<InternalCo
             lastKey = lastBucket.getRawKey();
         }
         reduceContext.consumeBucketsAndMaybeBreak(result.size());
-        return new InternalComposite(
+        InternalComposite reduced = new InternalComposite(
             name,
             size,
             sourceNames,
@@ -273,6 +272,8 @@ public class InternalComposite extends InternalMultiBucketAggregation<InternalCo
             earlyTerminated,
             metadata
         );
+        reduced.validateAfterKey();
+        return reduced;
     }
 
     @Override