siddharthteotia commented on code in PR #11496:
URL: https://github.com/apache/pinot/pull/11496#discussion_r1323314926
##########
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 let's get together on how to improve this in future. I think
@jasperjiaguo has already sync'd up with some of you offline on slack on that.
> I think we could just add a PinotProperty that defines the max dataTable
size and set that to something like 100-500 by default.
I think this will work but my concern is that it may lead to a lot of
pre-mature query killing and will introduce another config. Fan out is also a
factor that we should consider.
Let's all do some brainstorming separately.
--
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
