vvivekiyer commented on code in PR #16793:
URL: https://github.com/apache/pinot/pull/16793#discussion_r2369686568
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/QueryAggregator.java:
##########
@@ -298,28 +213,63 @@ public int getAggregationSleepTimeMs() {
return _sleepTime;
}
+ /**
+ * Get the current QueryMonitorConfig
+ */
+ public QueryMonitorConfig getQueryMonitorConfig() {
+ return _queryMonitorConfig.get();
+ }
+
+ /**
+ * Log the current QueryMonitorConfig settings
+ */
+ private void logQueryMonitorConfig() {
+ QueryMonitorConfig config = getQueryMonitorConfig();
+ LOGGER.info("Updated Configuration for Query Monitor");
+ LOGGER.info("Xmx is {}", config.getMaxHeapSize());
+ LOGGER.info("_instanceType is {}", _instanceType);
+ LOGGER.info("_alarmingLevel of on heap memory is {}",
config.getAlarmingLevel());
+ LOGGER.info("_criticalLevel of on heap memory is {}",
config.getCriticalLevel());
+ LOGGER.info("_panicLevel of on heap memory is {}", config.getPanicLevel());
+ LOGGER.info("_normalSleepTime is {}", config.getNormalSleepTime());
+ LOGGER.info("_alarmingSleepTime is {}", config.getAlarmingSleepTime());
+ LOGGER.info("_oomKillQueryEnabled: {}", config.isOomKillQueryEnabled());
+ LOGGER.info("_minMemoryFootprintForKill: {}",
config.getMinMemoryFootprintForKill());
+ LOGGER.info("_isCPUTimeBasedKillingEnabled: {},
_cpuTimeBasedKillingThresholdNS: {}",
+ config.isCpuTimeBasedKillingEnabled(),
config.getCpuTimeBasedKillingThresholdNS());
+ }
+
+ /**
+ * Register MSE cancel callback for graceful query termination
+ */
+ public void registerMseCancelCallback(String queryId, MseCancelCallback
callback) {
+ _queryCancelCallbacks.put(queryId, callback);
+ }
+
+ @Nullable
+ public MseCancelCallback getQueryCancelCallback(String queryId) {
+ return _queryCancelCallbacks.getIfPresent(queryId);
+ }
+
// Implement getQueryResources
public void
preAggregate(List<CPUMemThreadLevelAccountingObjects.ThreadEntry>
anchorThreadEntries) {
LOGGER.debug("Running pre-aggregate for QueryAggregator.");
- _sleepTime = _normalSleepTime;
- _triggeringLevel = TriggeringLevel.Normal;
+ QueryMonitorConfig config = getQueryMonitorConfig();
+ _sleepTime = config.getNormalSleepTime();
+ _aggregatedUsagePerActiveQuery = null;
collectTriggerMetrics();
evalTriggers();
if (_triggeringLevel == TriggeringLevel.HeapMemoryPanic) {
killAllQueries(anchorThreadEntries);
- LOGGER.error("Killed all queries and triggered gc!");
- // Set the triggering level back to normal for aggregation phase.
- _triggeringLevel = TriggeringLevel.Normal;
Review Comment:
If you don't set this back to normal, the aggregation phase trigger will
always fire?
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/QueryAggregator.java:
##########
@@ -67,74 +71,25 @@ enum TriggeringLevel {
private final HashMap<String, Long> _finishedTaskCPUStatsAggregator = new
HashMap<>();
private final HashMap<String, Long> _finishedTaskMemStatsAggregator = new
HashMap<>();
+ Cache<String, MseCancelCallback> _queryCancelCallbacks;
+
private final boolean _isThreadCPUSamplingEnabled;
private final boolean _isThreadMemorySamplingEnabled;
private final Set<String> _inactiveQuery;
+ private Set<String> _cancelSentQueries;
private final PinotConfiguration _config;
private final InstanceType _instanceType;
private final String _instanceId;
- // max heap usage, Xmx
- private final long _maxHeapSize = ResourceUsageUtils.getMaxHeapSize();
-
- // don't kill a query if its memory footprint is below some ratio of
_maxHeapSize
- private final long _minMemoryFootprintForKill;
-
- // kill all queries if heap usage exceeds this
- private final long _panicLevel;
-
- // kill the most expensive query if heap usage exceeds this
- private final long _criticalLevel;
-
- // if after gc the heap usage is still above this, kill the most expensive
query
- // use this to prevent heap size oscillation and repeatedly triggering gc
- private final long _criticalLevelAfterGC;
-
- // trigger gc if consecutively kill more than some number of queries
- // set this to 0 to always trigger gc before killing a query to give gc a
second chance
- // as would minimize the chance of false positive killing in some usecases
- // should consider use -XX:+ExplicitGCInvokesConcurrent to avoid STW for
some gc algorithms
- private final int _gcBackoffCount;
-
- // start to sample more frequently if heap usage exceeds this
- private final long _alarmingLevel;
-
- // normal sleep time
- private final int _normalSleepTime;
-
- // wait for gc to complete, according to system.gc() javadoc, when control
returns from the method call,
- // the Java Virtual Machine has made a best effort to reclaim space from all
discarded objects.
- // Therefore, we default this to 0.
- // Tested with Shenandoah GC and G1GC, with -XX:+ExplicitGCInvokesConcurrent
- private final int _gcWaitTime;
-
- // alarming sleep time denominator, should be > 1 to sample more frequent at
alarming level
- private final int _alarmingSleepTimeDenominator;
-
- // alarming sleep time
- private final int _alarmingSleepTime;
-
- // the framework would not commit to kill any query if this is disabled
- private final boolean _oomKillQueryEnabled;
-
- // if we want to publish the heap usage
- private final boolean _publishHeapUsageMetric;
-
- // if we want kill query based on CPU time
- private final boolean _isCPUTimeBasedKillingEnabled;
-
- // CPU time based killing threshold
- private final long _cpuTimeBasedKillingThresholdNS;
-
- private final boolean _isQueryKilledMetricEnabled;
+ // Centralized configuration using QueryMonitorConfig
+ private final AtomicReference<QueryMonitorConfig> _queryMonitorConfig = new
AtomicReference<>();
Review Comment:
Can we do a 1:1 comparison to make sure all these fields exist?
--
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]
