kfaraz commented on code in PR #16284:
URL: https://github.com/apache/druid/pull/16284#discussion_r1570316241


##########
server/src/main/java/org/apache/druid/indexing/overlord/supervisor/Supervisor.java:
##########
@@ -93,6 +94,14 @@ default Boolean isHealthy()
    */
   LagStats computeLagStats();
 
+  /**
+   * Used by AutoScaler to either scale by max/total/avg.
+   */
+  default LagMetric getLagMetricForAutoScaler()

Review Comment:
   This is a very roundabout and yet limiting manner of determining which lag 
metric to use for auto-scaling.
   
   We don't need the `LagMetric` class or the `Supervisor.getLagMetric` method.
   
   We can simply add another method `computeLagForAutoScaler()`. This clarifies 
the intent and is not confusing.
   
   ```java
   default long computeLagForAutoScaler() {
       LagStats lagStats = computeLagStats();
       return lagStats == null ? 0L : lagStats.maxLag();
   }
   ```
   
   Update the `LagBasedAutoScaler` to use this new method instead of 
`computeLagStats()`.
   
   cc: @adithyachakilam , @AmatyaAvadhanula 



-- 
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]

Reply via email to