SabrinaZhaozyf commented on code in PR #11942:
URL: https://github.com/apache/pinot/pull/11942#discussion_r1385691506
##########
pinot-core/src/main/java/org/apache/pinot/core/query/scheduler/QueryScheduler.java:
##########
@@ -215,6 +219,14 @@ private byte[] serializeResponse(ServerQueryRequest
queryRequest, InstanceRespon
byte[] responseByte = null;
try {
responseByte = instanceResponse.toDataTable().toBytes();
Review Comment:
> Could we verify whether there's memory allocation in toBytes()? Do we need
to instrument that part as well?
Good point. There's memory allocation in toBytes (mostly from
`writeLeadingSections`) and we have seen OOM here as well. Added
instrumentation to
[serializeStringDictionary](https://github.com/apache/pinot/blob/9092244e0be9f27158320c987833bdb3b6179bdd/pinot-common/src/main/java/org/apache/pinot/common/datatable/DataTableImplV4.java#L332)
in toBytes.
> IMO it might be helpful to very briefly explain why we only added
instrumentation only for groupby and selection operation utils in the PR
description
Done.
--
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
