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 event is
very seldom, so when it happens we want to make sure it first to 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. 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: [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]