This is an automated email from the ASF dual-hosted git repository. jackie pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/pinot.git
The following commit(s) were added to refs/heads/master by this push: new a208607254 [#10884][feature] add requestId for BrokerResponse in pinot-broker and java-client (#10943) a208607254 is described below commit a2086072545fc67a35f5d6756c3b9efc97d15641 Author: mingmxu <mingm...@gmail.com> AuthorDate: Thu Jun 22 23:49:25 2023 -0700 [#10884][feature] add requestId for BrokerResponse in pinot-broker and java-client (#10943) --- .../requesthandler/BaseBrokerRequestHandler.java | 1 + .../org/apache/pinot/client/BrokerResponse.java | 6 +++ .../apache/pinot/client/BrokerResponseTest.java | 52 ++++++++++++++++++++++ .../org/apache/pinot/client/aggregation.json | 1 + .../apache/pinot/client/aggregationGroupBy.json | 1 + .../org/apache/pinot/client/exception.json | 1 + .../org/apache/pinot/client/selection.json | 1 + .../pinot/common/response/BrokerResponse.java | 10 +++++ .../response/broker/BrokerResponseNative.java | 15 ++++++- .../response/broker/BrokerResponseNativeV2.java | 10 ----- .../response/broker/BrokerResponseStats.java | 2 +- 11 files changed, 88 insertions(+), 12 deletions(-) diff --git a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java index 63089e863c..406734812d 100644 --- a/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java +++ b/pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java @@ -262,6 +262,7 @@ public abstract class BaseBrokerRequestHandler implements BrokerRequestHandler { } } + brokerResponse.setRequestId(String.valueOf(requestId)); return brokerResponse; } diff --git a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerResponse.java b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerResponse.java index 8f06afab04..c8cd0b8ce3 100644 --- a/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerResponse.java +++ b/pinot-clients/pinot-java-client/src/main/java/org/apache/pinot/client/BrokerResponse.java @@ -25,6 +25,7 @@ import com.fasterxml.jackson.databind.JsonNode; * Reimplementation of BrokerResponse from pinot-common, so that pinot-api does not depend on pinot-common. */ public class BrokerResponse { + private String _requestId; private JsonNode _aggregationResults; private JsonNode _selectionResults; private JsonNode _resultTable; @@ -35,6 +36,7 @@ public class BrokerResponse { } private BrokerResponse(JsonNode brokerResponse) { + _requestId = brokerResponse.get("requestId") != null ? brokerResponse.get("requestId").asText() : "unknown"; _aggregationResults = brokerResponse.get("aggregationResults"); _exceptions = brokerResponse.get("exceptions"); _selectionResults = brokerResponse.get("selectionResults"); @@ -81,4 +83,8 @@ public class BrokerResponse { static BrokerResponse empty() { return new BrokerResponse(); } + + public String getRequestId() { + return _requestId; + } } diff --git a/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/BrokerResponseTest.java b/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/BrokerResponseTest.java new file mode 100644 index 0000000000..219c7a0948 --- /dev/null +++ b/pinot-clients/pinot-java-client/src/test/java/org/apache/pinot/client/BrokerResponseTest.java @@ -0,0 +1,52 @@ +/** + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.pinot.client; + +import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.ObjectReader; +import org.apache.pinot.spi.utils.JsonUtils; +import org.testng.Assert; +import org.testng.annotations.Test; + + +public class BrokerResponseTest { + private static final ObjectReader OBJECT_READER = JsonUtils.DEFAULT_READER; + + @Test + public void parseResultWithRequestId() + throws JsonProcessingException { + String responseJson = "{\"requestId\":\"1\",\"traceInfo\":{},\"numDocsScanned\":36542," + + "\"aggregationResults\":[{\"function\":\"count_star\",\"value\":\"36542\"}],\"timeUsedMs\":30," + + "\"segmentStatistics\":[],\"exceptions\":[],\"totalDocs\":115545}"; + BrokerResponse brokerResponse = BrokerResponse.fromJson(OBJECT_READER.readTree(responseJson)); + Assert.assertEquals("1", brokerResponse.getRequestId()); + Assert.assertTrue(!brokerResponse.hasExceptions()); + } + + @Test + public void parseResultWithoutRequestId() + throws JsonProcessingException { + String responseJson = "{\"traceInfo\":{},\"numDocsScanned\":36542," + + "\"aggregationResults\":[{\"function\":\"count_star\",\"value\":\"36542\"}],\"timeUsedMs\":30," + + "\"segmentStatistics\":[],\"exceptions\":[],\"totalDocs\":115545}"; + BrokerResponse brokerResponse = BrokerResponse.fromJson(OBJECT_READER.readTree(responseJson)); + Assert.assertEquals("unknown", brokerResponse.getRequestId()); + Assert.assertTrue(!brokerResponse.hasExceptions()); + } +} diff --git a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregation.json b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregation.json index ff9a775e74..511b704049 100644 --- a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregation.json +++ b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregation.json @@ -1,4 +1,5 @@ { + "requestId": "1", "traceInfo": {}, "numDocsScanned": 36542, "aggregationResults": [ diff --git a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregationGroupBy.json b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregationGroupBy.json index bfe0b26c25..b589cef4c0 100644 --- a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregationGroupBy.json +++ b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/aggregationGroupBy.json @@ -1,4 +1,5 @@ { + "requestId": "1", "traceInfo": {}, "numDocsScanned": 22598, "aggregationResults": [ diff --git a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/exception.json b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/exception.json index bc7c99ccb5..7a4e7e4fd5 100644 --- a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/exception.json +++ b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/exception.json @@ -1,4 +1,5 @@ { + "requestId": "1", "traceInfo": {}, "numDocsScanned": 0, "aggregationResults": [], diff --git a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/selection.json b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/selection.json index e3068dc263..7825e87620 100644 --- a/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/selection.json +++ b/pinot-clients/pinot-java-client/src/test/resources/org/apache/pinot/client/selection.json @@ -1,4 +1,5 @@ { + "requestId": "1", "selectionResults": { "columns": [ "ActualElapsedTime", diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/BrokerResponse.java b/pinot-common/src/main/java/org/apache/pinot/common/response/BrokerResponse.java index e50ea2b2ee..3ad49460bc 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/BrokerResponse.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/BrokerResponse.java @@ -325,4 +325,14 @@ public interface BrokerResponse { * Set the total number of segments with a MatchAllFilterOperator when Explain Plan is called */ void setExplainPlanNumMatchAllFilterSegments(long explainPlanNumMatchAllFilterSegments); + + /** + * get request ID for the query + */ + String getRequestId(); + + /** + * set request ID generated by broker + */ + void setRequestId(String requestId); } diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java index 76ed75a466..f2580b9cc3 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNative.java @@ -41,7 +41,7 @@ import org.apache.pinot.spi.utils.JsonUtils; * Supports serialization via JSON. */ @JsonPropertyOrder({ - "resultTable", "exceptions", "numServersQueried", "numServersResponded", "numSegmentsQueried", + "resultTable", "requestId", "exceptions", "numServersQueried", "numServersResponded", "numSegmentsQueried", "numSegmentsProcessed", "numSegmentsMatched", "numConsumingSegmentsQueried", "numConsumingSegmentsProcessed", "numConsumingSegmentsMatched", "numDocsScanned", "numEntriesScannedInFilter", "numEntriesScannedPostFilter", "numGroupsLimitReached", "totalDocs", "timeUsedMs", "offlineThreadCpuTimeNs", "realtimeThreadCpuTimeNs", @@ -57,6 +57,7 @@ public class BrokerResponseNative implements BrokerResponse { new BrokerResponseNative(QueryException.TABLE_DOES_NOT_EXIST_ERROR); public static final BrokerResponseNative BROKER_ONLY_EXPLAIN_PLAN_OUTPUT = getBrokerResponseExplainPlanOutput(); + private String _requestId; private int _numServersQueried = 0; private int _numServersResponded = 0; private long _numDocsScanned = 0L; @@ -557,4 +558,16 @@ public class BrokerResponseNative implements BrokerResponse { public int getExceptionsSize() { return _processingExceptions.size(); } + + @JsonProperty("requestId") + @Override + public String getRequestId() { + return _requestId; + } + + @JsonProperty("requestId") + @Override + public void setRequestId(String requestId) { + _requestId = requestId; + } } diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNativeV2.java b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNativeV2.java index 8465d47f81..95943881dc 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNativeV2.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseNativeV2.java @@ -46,7 +46,6 @@ import org.apache.pinot.spi.utils.JsonUtils; "realtimeTotalCpuTimeNs", "segmentStatistics", "traceInfo" }) public class BrokerResponseNativeV2 extends BrokerResponseNative { - private String _requestId; private final Map<Integer, BrokerResponseStats> _stageIdStats = new HashMap<>(); @@ -93,13 +92,4 @@ public class BrokerResponseNativeV2 extends BrokerResponseNative { public Map<Integer, BrokerResponseStats> getStageIdStats() { return _stageIdStats; } - - @JsonProperty("requestId") - public String getRequestId() { - return _requestId; - } - - public void setRequestId(String requestId) { - _requestId = requestId; - } } diff --git a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseStats.java b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseStats.java index 23482e905c..f2361a7908 100644 --- a/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseStats.java +++ b/pinot-common/src/main/java/org/apache/pinot/common/response/broker/BrokerResponseStats.java @@ -33,7 +33,7 @@ import org.apache.pinot.spi.utils.JsonUtils; // same metadataKey // TODO: Replace member fields with a simple map of <MetadataKey, Object> // TODO: Add a subStat field, stage level subStats will contain each operator stats -@JsonPropertyOrder({"exceptions", "numBlocks", "numRows", "stageExecutionTimeMs", "stageExecutionUnit", +@JsonPropertyOrder({"requestId", "exceptions", "numBlocks", "numRows", "stageExecutionTimeMs", "stageExecutionUnit", "stageExecWallTimeMs", "stageExecEndTimeMs", "numServersQueried", "numServersResponded", "numSegmentsQueried", "numSegmentsProcessed", "numSegmentsMatched", "numConsumingSegmentsQueried", "numConsumingSegmentsProcessed", "numConsumingSegmentsMatched", "numDocsScanned", "numEntriesScannedInFilter", "numEntriesScannedPostFilter", --------------------------------------------------------------------- To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org For additional commands, e-mail: commits-h...@pinot.apache.org