jasperjiaguo commented on code in PR #10373:
URL: https://github.com/apache/pinot/pull/10373#discussion_r1124924913


##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -724,14 +745,14 @@ void killAllQueries() {
        */
       private void killMostExpensiveQuery() {
         if (!_aggregatedUsagePerActiveQuery.isEmpty() && 
_numQueriesKilledConsecutively >= _gcTriggerCount) {
-          System.gc();
           _numQueriesKilledConsecutively = 0;
+          System.gc();
           try {
-            Thread.sleep(_normalSleepTime);
+            Thread.sleep(_gcWaitTime);
           } catch (InterruptedException ignored) {
           }
           _usedBytes = MEMORY_MX_BEAN.getHeapMemoryUsage().getUsed();
-          if (_usedBytes < _criticalLevel) {
+          if (_usedBytes < _criticalLevelAfterGC) {

Review Comment:
   Yes that is expected to prevent heap size oscillation and repetitive gc 
triggering. So in production this code path is almost never triggered as 
`_numQueriesKilledConsecutively` is mostly at a low level and 
`_numQueriesKilledConsecutively >= _gcTriggerCount` is false. We now want to 
also use this path to "give gc a second chance" if it hasn't kick in by setting 
`_gcTriggerCount` to 0. For these use cases, even HeapMemoryCritical exceed is 
very seldom, so when it happens we want to make sure it first do a gc. What we 
observed there is once the gc happens it would collect a ton of garbage. So if 
it fails to do so (`_usedBytes > _criticalLevelAfterGC`) we should still kill 
the query, incase in the next iteration the heap exceeds again and we trigger 
another gc. Nevertheless, this _criticalLevelAfterGC itself is configurable so 
we can make `_criticalLevelAfterGC= _criticalLevel` if we want the detection to 
be even less sensitive.



-- 
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

Reply via email to