You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@skywalking.apache.org by GitBox <gi...@apache.org> on 2022/04/28 01:30:35 UTC

[GitHub] [skywalking] kezhenxu94 commented on a diff in pull request #8959: Improve the performance of TermsAggregation

kezhenxu94 commented on code in PR #8959:
URL: https://github.com/apache/skywalking/pull/8959#discussion_r860376121


##########
oap-server/server-library/library-elasticsearch-client/src/main/java/org/apache/skywalking/library/elasticsearch/requests/search/aggregation/TermsAggregation.java:
##########
@@ -58,6 +58,8 @@ public void serialize(final TermsAggregation value, final JsonGenerator gen,
                     if (value.getOrder() != null) {
                         writeOrder(value, gen);
                     }
+                    gen.writeStringField("execution_hint", "map");
+                    gen.writeStringField("collect_mode", "breadth_first");

Review Comment:
   Please don't do this in the codec level.
   
   - Add fields `executionHint`, `collectMode` in `TermsAggregation` (you can make them enum type considering their possible values are limited in ES's spec).
   - Add fields `executionHint`, `collectMode` in `TermsAggregationBuilder` and corresponding builder methods, pass the two to `TermsAggregation` constructor in `build` method.
   - Write the 2 fields here only when they are not empty/blank.
   
   Then you should change set the 2 fields in topology query.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscribe@skywalking.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org