siddharthteotia commented on a change in pull request #6680:
URL: https://github.com/apache/incubator-pinot/pull/6680#discussion_r595676976
##########
File path:
pinot-core/src/main/java/org/apache/pinot/core/operator/blocks/InstanceResponseBlock.java
##########
@@ -41,6 +41,9 @@
public InstanceResponseBlock(IntermediateResultsBlock
intermediateResultsBlock) {
try {
_instanceResponseDataTable = intermediateResultsBlock.getDataTable();
+ long totalThreadCpuTimeNs =
intermediateResultsBlock.getThreadCpuTimeNs();
+
_instanceResponseDataTable.getMetadata().put(DataTable.THREAD_CPU_TIME_NS_METADATA_KEY,
Review comment:
I think logging should still be done in QueryScheduler since that's
where all the metrics of a query are logged together in one line once the
processing is done. So, we shouldn't log it in a new place now imo.
As you suggested, I think we can do a put() here, retrieve the metric in
QueryScheduler from dataTable metadata and then make sure to do a remove()
before `byte[] responseData = serializeDataTable(queryRequest, dataTable);` is
called. Use the retrieved value for adding to table gauge and writing the log.
For the remove(), add a TODO to revisit this when follow-up work of data table
serialization cost measurement is done. Should be fine since we are going to do
the follow-up immediately after this.
----------------------------------------------------------------
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.
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
