vvivekiyer commented on code in PR #15798:
URL: https://github.com/apache/pinot/pull/15798#discussion_r2190769293
##########
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:
Initialized it here because it needs to be accessed from QuerySchedulers and
other places. Of course it can be moved to WorkloadAggregator and indirectly
accessed from there. But given that we are going to eventually replace
`PerQuery..Acountant`, it makes sense to keep it here?
##########
pinot-spi/src/main/java/org/apache/pinot/spi/accounting/ThreadResourceUsageAccountant.java:
##########
@@ -50,16 +50,26 @@ void createExecutionContext(String queryId, int taskId,
ThreadExecutionContext.T
* @param taskId a unique task id
* @param taskType the type of the task - SSE or MSE
*/
+ @Deprecated
Review Comment:
workloadName can never be null. QueryOptionUtils::getWorkloadName always
returns a default workload name.
--
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]
