vrajat commented on code in PR #16172:
URL: https://github.com/apache/pinot/pull/16172#discussion_r2163757738
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -482,6 +482,17 @@ public Map<String, AggregatedStats> aggregate(boolean
isTriggered) {
public void postAggregation(Map<String, AggregatedStats>
aggregatedUsagePerActiveQuery) {
}
+ protected void logQueryResourceUsage(Map<String, ? extends
QueryResourceTracker> aggregatedUsagePerActiveQuery) {
+ LOGGER.warn("Current task status recorded is {}", _threadEntriesMap);
+ LOGGER.warn("Query aggregation results {} for the previous kill.",
aggregatedUsagePerActiveQuery.toString());
Review Comment:
This is the same as before.
`_threadEntriesMap` is not useful in my experience. Query Aggregation
results have been useful to know the other queries running in the server. I'll
remove the log line for `_threadEntriesMap`. Tell me if you want it back.
I consolidate the following log lines into this function.
https://github.com/apache/pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java#L897
https://github.com/apache/pinot/blob/master/pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java#L923
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -482,6 +482,17 @@ public Map<String, AggregatedStats> aggregate(boolean
isTriggered) {
public void postAggregation(Map<String, AggregatedStats>
aggregatedUsagePerActiveQuery) {
}
+ protected void logQueryResourceUsage(Map<String, ? extends
QueryResourceTracker> aggregatedUsagePerActiveQuery) {
Review Comment:
Good point. I'll not log queries at all. This will be similar to the current
state.
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -482,6 +482,17 @@ public Map<String, AggregatedStats> aggregate(boolean
isTriggered) {
public void postAggregation(Map<String, AggregatedStats>
aggregatedUsagePerActiveQuery) {
}
+ protected void logQueryResourceUsage(Map<String, ? extends
QueryResourceTracker> aggregatedUsagePerActiveQuery) {
+ LOGGER.warn("Current task status recorded is {}", _threadEntriesMap);
+ LOGGER.warn("Query aggregation results {} for the previous kill.",
aggregatedUsagePerActiveQuery.toString());
+ }
+
+ protected void logTerminatedQuery(QueryResourceTracker
queryResourceTracker, long totalHeapMemoryUsage) {
Review Comment:
We dont want these logs. We have a Postgres system table inspired logging
framework. So we want to log this info into that system.
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
