suneet-s commented on code in PR #17356:
URL: https://github.com/apache/druid/pull/17356#discussion_r1802073660
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -500,6 +502,7 @@ public void handle()
boolean allocationSuccess = changeTaskCount(desiredTaskCount);
if (allocationSuccess) {
+ actionAfterScale.run();
Review Comment:
nit:
```suggestion
onSuccessfulScale.run();
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -1260,9 +1263,13 @@ public void tryInit()
}
}
- public Runnable buildDynamicAllocationTask(Callable<Integer> scaleAction,
ServiceEmitter emitter)
+ public Runnable buildDynamicAllocationTask(
+ Callable<Integer> scaleAction,
Review Comment:
nit:
```suggestion
Callable<Integer> computeDesiredTaskCount,
```
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisor.java:
##########
@@ -500,6 +502,7 @@ public void handle()
boolean allocationSuccess = changeTaskCount(desiredTaskCount);
Review Comment:
Would it make sense to move the metric emission of desired task count to
line 505 so that the desired task count is only reported when changing task
count is successful? The check on desiredTaskCount > 0 looks similar to the
check in the `chageTaskCount` method
--
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]
