Copilot commented on code in PR #16647:
URL: https://github.com/apache/pinot/pull/16647#discussion_r2289116287
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -832,15 +832,20 @@ protected void evalTriggers() {
}
break;
case CPUTimeBasedKilling:
+ LOGGER.info("Entering CPU time based killing mode, killing
queries that exceed threshold (ns): {}",
+ config.getCpuTimeBasedKillingThresholdNS());
if (!_isThreadCPUSamplingEnabled) {
LOGGER.error("Unable to terminate queries as CPU time tracking
is not enabled");
}
break;
case HeapMemoryAlarmingVerbose:
LOGGER.debug("Heap used bytes: {} exceeds alarming level: {}",
_usedBytes, config.getAlarmingLevel());
break;
- default:
+ case Normal:
+ LOGGER.info("Heap used bytes: {} drops to safe zone, entering to
normal mode", _usedBytes);
break;
+ default:
+ throw new IllegalStateException("Unsupported triggering level: "
+ _triggeringLevel);
Review Comment:
Adding a throwing default case in a switch statement that previously had a
silent default could be a breaking change if there are legitimate triggering
levels not handled by the existing cases. Consider logging a warning instead of
throwing an exception, or ensure all possible enum values are explicitly
handled.
```suggestion
LOGGER.warn("Encountered unsupported triggering level: {}",
_triggeringLevel);
```
--
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]
