Jackie-Jiang commented on code in PR #11710:
URL: https://github.com/apache/pinot/pull/11710#discussion_r1361236279
##########
pinot-common/src/main/java/org/apache/pinot/common/metrics/BrokerMeter.java:
##########
@@ -99,7 +99,8 @@ public enum BrokerMeter implements AbstractMetrics.Meter {
NETTY_CONNECTION_BYTES_RECEIVED("nettyConnection", true),
PROACTIVE_CLUSTER_CHANGE_CHECK("proactiveClusterChangeCheck", true),
- DIRECT_MEMORY_OOM("directMemoryOOMCount", true);
+ DIRECT_MEMORY_OOM("directMemoryOOMCount", true),
+ MAX_QUERY_RESPONSE_SIZE("maxQueryResponseSize", true);
Review Comment:
I don't see much value for this metric
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -682,6 +682,24 @@ protected BrokerResponse handleRequest(long requestId,
String query, @Nullable S
return new BrokerResponseNative(exceptions);
}
+ // Set the maximum serialized response size per server
+ int numServers = 0;
+ if (offlineRoutingTable != null) {
+ numServers += offlineRoutingTable.keySet().size();
Review Comment:
(minor)
```suggestion
numServers += offlineRoutingTable.size();
```
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -682,6 +682,24 @@ protected BrokerResponse handleRequest(long requestId,
String query, @Nullable S
return new BrokerResponseNative(exceptions);
}
+ // Set the maximum serialized response size per server
Review Comment:
(minor) Wrong indentation
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -682,6 +682,24 @@ protected BrokerResponse handleRequest(long requestId,
String query, @Nullable S
return new BrokerResponseNative(exceptions);
}
+ // Set the maximum serialized response size per server
+ int numServers = 0;
+ if (offlineRoutingTable != null) {
+ numServers += offlineRoutingTable.keySet().size();
+ }
+ if (realtimeRoutingTable != null) {
+ numServers += realtimeRoutingTable.keySet().size();
Review Comment:
(minor)
```suggestion
numServers += realtimeRoutingTable.size();
```
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -1654,6 +1672,74 @@ private long setQueryTimeout(String tableNameWithType,
Map<String, String> query
return remainingTimeMs;
}
+ /**
+ * Sets a query option indicating the maximum response size that can be sent
from a server to the broker. This size
+ * is measured for the serialized response.
+ */
+ private void setMaxServerResponseSizeBytes(int numServers, Map<String,
String> queryOptions,
+ TableConfig tableConfig) {
+ if (numServers == 0) {
+ return;
+ }
+
+ // The overriding order of priority is:
+ // 1. QueryOption -> maxServerResponseSizeBytes
+ // 2. QueryOption -> maxQueryResponseSizeBytes
+ // 3. TableConfig -> maxServerResponseSizeBytes
+ // 4. TableConfig -> maxQueryResponseSizeBytes
+ // 5. BrokerConfig -> maxServerResponseSizeBytes
+ // 6. BrokerConfig -> maxServerResponseSizeBytes
+
+ // QueryOption
+ if (QueryOptionsUtils.getMaxServerResponseSizeBytes(queryOptions) != null)
{
+ return;
+ }
+ if (QueryOptionsUtils.getMaxQueryResponseSizeBytes(queryOptions) != null) {
Review Comment:
(minor) Put this in a local variable to avoid reading it twice
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
