vvivekiyer commented on code in PR #11710:
URL: https://github.com/apache/pinot/pull/11710#discussion_r1343207537
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -170,15 +171,18 @@ public BaseBrokerRequestHandler(PinotConfiguration
config, String brokerId, Brok
_brokerTimeoutMs = config.getProperty(Broker.CONFIG_OF_BROKER_TIMEOUT_MS,
Broker.DEFAULT_BROKER_TIMEOUT_MS);
_queryResponseLimit =
config.getProperty(Broker.CONFIG_OF_BROKER_QUERY_RESPONSE_LIMIT,
Broker.DEFAULT_BROKER_QUERY_RESPONSE_LIMIT);
+ _queryMaxSerializedResponseBytes =
config.getProperty(Broker.CONFIG_OF_MAX_QUERY_RESPONSE_SIZE_BYTES,
Review Comment:
Since this is a instance config that doesn't change unless restarted, I've
actually added a log to indicate this value. I can add a gauge metric for sure,
but do you think it'll be helpful?
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -1654,6 +1669,24 @@ 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 setServerSerializedResponseLength(int numServers, @Nullable
BrokerRequest brokerRequest) {
+ if (brokerRequest == null) {
+ return;
+ }
+
+ Map<String, String> queryOptions =
brokerRequest.getPinotQuery().getQueryOptions();
+ Long maxLength =
QueryOptionsUtils.getMaxSerializedResponseLengthPerServer(queryOptions);
Review Comment:
There are 2 configs involved here:
There's a broker instance config that defines what the maximum response size
for single query can be. If this is set, this total value is divided by the
number of routed servers to determine the `per_server_budget`. This
`per_server_budge` is set as a query config very similar to how we handle
timeout today (the alternative is to make thrift changes to pass this info).
##########
pinot-broker/src/main/java/org/apache/pinot/broker/requesthandler/BaseBrokerRequestHandler.java:
##########
@@ -1654,6 +1669,24 @@ 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 setServerSerializedResponseLength(int numServers, @Nullable
BrokerRequest brokerRequest) {
+ if (brokerRequest == null) {
+ return;
+ }
+
+ Map<String, String> queryOptions =
brokerRequest.getPinotQuery().getQueryOptions();
+ Long maxLength =
QueryOptionsUtils.getMaxSerializedResponseLengthPerServer(queryOptions);
+ if (maxLength == null && _queryMaxSerializedResponseBytes > 0) {
+ long maxLengthPerServer = _queryMaxSerializedResponseBytes / numServers;
Review Comment:
This is not possible today as we check for zero servers in L651.
Nevertheless, I added a safeguard condition.
--
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: commits-unsubscr...@pinot.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscr...@pinot.apache.org
For additional commands, e-mail: commits-h...@pinot.apache.org
