ege-st commented on code in PR #11496:
URL: https://github.com/apache/pinot/pull/11496#discussion_r1322163994
##########
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:
> I think instead of a warning, emitting a metric may be much better to help
us plan future enhancements IMO.
Yes, this is why I'd like to have a histogram metric for the size of message
sent from the Server to a Broker. Then we can figure out a reliable max size
over a period of time where this OOM does not happen and use that for setting a
max size threshold.
My opinion here is that there are several ways that we can have a more
elegant solution but dealing with the Direct Buffer OOM is still a fundamental
requirement we should have. And I think the right first step is to have a way
to deal with that DM OOM and collect additional data about what we are sending
between nodes, then use that data to understand how how to tune the more
nuanced but more elegant next steps.
--
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
