kfaraz commented on code in PR #18976:
URL: https://github.com/apache/druid/pull/18976#discussion_r2764143457
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/autoscaler/CostBasedAutoScaler.java:
##########
@@ -88,9 +71,11 @@ public class CostBasedAutoScaler implements
SupervisorTaskAutoScaler
private final ServiceMetricEvent.Builder metricBuilder;
private final ScheduledExecutorService autoscalerExecutor;
private final WeightedCostFunction costFunction;
+ private OptimalTaskCountBoundariesPlugin boundariesPlugin = null;
Review Comment:
This is nice! I like the plugin approach.
Couple of suggestions:
- Can we think of each plugin as being a `CostFunction` itself?
- The `WeightedCostFunction` could also implement the `CostFunction`
interface.
- The `CostFunction` interface would have a single method:
```java
CostResult computeCost(
CostMetrics metrics,
int proposedTaskCount,
CostBasedAutoScalerConfig config
);
```
- Then we could do something like `f(g(h(weightedCost())))`, where `f`, `g`
and `h` are various plugins on top of the cost function.
```java
CostFunction costFunction = new WeightedCostFunction();
if (burstEnabled) {
costFunction = new BurstFunction(costFunction);
}
if (taskLimitEnabled) {
costFunction = new TaskBoundariesFunction(costFunction);
}
CostResult costResult = costFunction.computeCost();
```
- This approach would allow `WeightedCostFunction` to remain agnostic of all
plugins
--
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]
