gortiz commented on code in PR #11496:
URL: https://github.com/apache/pinot/pull/11496#discussion_r1325471590
##########
pinot-core/src/main/java/org/apache/pinot/core/transport/InstanceRequestHandler.java:
##########
@@ -321,6 +322,10 @@ private void sendResponse(ChannelHandlerContext ctx,
String tableNameWithType, l
LOGGER.info("Slow query: request handler processing time: {}, send
response latency: {}, total time to handle "
+ "request: {}", queryProcessingTimeMs, sendResponseLatencyMs,
totalQueryTimeMs);
}
+ if (serializedDataTable.length > LARGE_RESPONSE_SIZE_THRESHOLD_BYTES) {
+ LOGGER.warn("Large query: response size in bytes: {}, table name {}",
+ serializedDataTable.length, tableNameWithType);
Review Comment:
> yes, the risk with the message size threshold is that if we are not very
careful in tuning we'll start killing previously fine queries. Hence the need
for a histogram metric to get an idea of the size of the messages.
We have that metric. See `BrokerMeter.NETTY_CONNECTION_BYTES_SENT`,
`BrokerMeter.NETTY_CONNECTION_BYTES_RECEIVED` and similar meters in servers.
Also, the PR I linked adds see `ServerGauge.NETTY_POOLED_USED_DIRECT_MEMORY`
and `BrokerGauge.NETTY_POOLED_USED_DIRECT_MEMORY` (but the once we already have
are the one you asking for).
Also, we have other similar restrictions (like the implicit limit we have in
queries) and we didn't make a super-large study to know the exact number. It is
enough to just add heuristic number and let the users change them with a config.
--
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
