vrajat commented on code in PR #15798:
URL: https://github.com/apache/pinot/pull/15798#discussion_r2192107249
##########
pinot-core/src/main/java/org/apache/pinot/core/accounting/PerQueryCPUMemAccountantFactory.java:
##########
@@ -314,12 +330,21 @@ public void sampleThreadBytesAllocated() {
}
}
+ @Deprecated
@Override
- public void setupRunner(@Nullable String queryId, int taskId,
ThreadExecutionContext.TaskType taskType) {
+ public void setupRunner(String queryId, int taskId,
ThreadExecutionContext.TaskType taskType) {
+ setupRunner(queryId, taskId, taskType, null);
Review Comment:
If `workloadName` should never be null (mentioned in other comments), then
the last para, should be default workload name ?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/trace/Tracing.java:
##########
@@ -301,6 +327,7 @@ public static void clear() {
public static void initializeThreadAccountant(PinotConfiguration config,
String instanceId,
InstanceType instanceType) {
String factoryName =
config.getProperty(CommonConstants.Accounting.CONFIG_OF_FACTORY_NAME);
+ _workloadBudgetManager = new WorkloadBudgetManager(config);
Review Comment:
I'll start an offline thread about deprecating `PerQuery...`. Even if it is
removed, the only place it is used is in `WorkloadAggregator`. Will it be used
in other places ? Right now, the code is laid such that SPI only focuses on
accounting. `startWatcherTask` is the only exception (and unnecessary in
hindsight). Workload and budget management are implementation dependent and
seems like core package is better place.
It seems like we will be using `PerQuery...` for sometime. Initializing
workload budget manager is not required for it. Even if we move away from
`PerQuery...`, there is a good chance that we may adopt a system that doesnt
use workload budget manager. The option to not used should be available.
--
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]
