siddharthteotia commented on a change in pull request #6680:
URL: https://github.com/apache/incubator-pinot/pull/6680#discussion_r595643785
##########
File path:
pinot-common/src/main/java/org/apache/pinot/common/metrics/ServerMeter.java
##########
@@ -65,6 +65,7 @@
SEGMENT_DOWNLOAD_FAILURES("segments", false),
NUM_RESIZES("numResizes", false),
RESIZE_TIME_MS("resizeTimeMs", false),
+ THREAD_CPU_TIME_NS("threadCpuTimeNs", false),
Review comment:
Thinking more about this
We have ServerMeter, ServerGauge, ServerTimer and ServerQueryPhase.
- This metric doesn't fit into ServerQueryPhase since we are not introducing
a new phase.
- Similarly, ServerMeter is also not right as it is not used for emitting
duration related metrics.
So between ServerGauge and ServerTimer, I am thinking we should use
ServerTimer. Currently it is used for freshness lag and netty server response
latency. We can introduce a new timer for CPU time measurements and then use
`serverMetrics.addTimedTableValue(tableNameWithType,
ServerTimer.THREAD_CPU_TIME ....)`
Now for the naming, we currently use `THREAD_CPU_TIME_NS`. Right now our
focus is on two main costs for query execution
- Execution for segment level and combine - this PR
- DataTable serialization - next PR
For sending to the broker, it can add up both, but I feel we should have
different metric names for cpu timers for the server to emit both separately.
So, something like EXECUTION_THREAD_CPU_TIME_NS and
RESPONSE_SERIALIZATION_CPU_TIME_NS.
Secondly, in future we might decide to emit the metrics for let's say
pruning, physical planning etc. Server can continue to add up everything and
send to broker, but I think it should still emit individually for which we need
separate names
@mcvsubbu @mqliang what do you think ?
----------------------------------------------------------------
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:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
