FrankChen021 commented on code in PR #19655:
URL: https://github.com/apache/druid/pull/19655#discussion_r3674252862


##########
docs/ingestion/supervisor.md:
##########
@@ -208,13 +208,13 @@ The following table outlines the configuration properties 
related to the `costBa
 
 | Property | Description | Required | Default                   |
 |----------|-------------|----------|---------------------------|
-|`scaleActionPeriodMillis`|How often, in milliseconds, Druid evaluates whether 
to scale.|No| `600000` (10 min)         |
+|`scaleActionPeriodMillis`|How often, in milliseconds, Druid evaluates whether 
to scale.|No| `120000` (2 min)          |
 |`lagWeight`|How much weight to give the lag cost relative to the idle cost. 
Higher values make the autoscaler more aggressive about adding tasks to drain 
backlog.|No| `0.4`                     |
 |`idleWeight`|How much weight to give the idle cost relative to the lag cost. 
Higher values make the autoscaler more aggressive about removing 
over-provisioned tasks.|No| `0.6`                     |
 |`useTaskCountBoundariesOnScaleUp`|Limits scale-up to a small step relative to 
the current task count, preventing large jumps. Disable to allow the autoscaler 
to jump directly to any task count.|No| `false`                   |
 |`useTaskCountBoundariesOnScaleDown`|Limits scale-down to a small step 
relative to the current task count, preventing large drops. Disable to allow 
the autoscaler to drop directly to any task count.|No| `true`                   
 |
-|`minScaleUpDelay`|Minimum cooldown after a scale-up before the next scale-up 
is allowed. Specified as an ISO-8601 duration.|No| `scaleActionPeriodMillis` |
-|`minScaleDownDelay`|Minimum cooldown after a scale-down before the next 
scale-down is allowed. Specified as an ISO-8601 duration.|No| `PT30M`           
        |
+|`minScaleUpDelay`|Minimum cooldown after a scale-up before the next scale-up 
is allowed. Specified as an ISO-8601 duration.|No| `PT15M`                   |
+|`minScaleDownDelay`|Minimum cooldown after a scale-down before the next 
scale-down is allowed. Specified as an ISO-8601 duration.|No| `PT20M`           
        |

Review Comment:
   [P2] Document the new emergency controls
   
   The cost-based autoscaler reference still omits both new operator-facing 
properties, `criticalLagThreshold` and `criticalLagAmplificationMultiplier`. 
Since the threshold defaults to null, readers cannot discover how to enable the 
headline feature or that it uses aggregate lag; the configurable multiplier 
also makes the fixed-6.0 wording in the Javadoc and PR description stale. 
Please document the properties, aggregate-lag units, 75%/100% tier behavior, 
and null/6.0 defaults.



##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/autoscaler/CostBasedAutoScaler.java:
##########
@@ -292,6 +329,12 @@ int computeOptimalTaskCount(CostMetrics metrics)
                    : startIndex;
     }
 
+    // Emergency (tier 2) lag skips the argmin search entirely: evaluate only 
the maximum valid task count.
+    if (emergencyLag) {

Review Comment:
   Rechecked on the current head: this is addressed. Tier 2 now selects the 
maximum valid task count even when that candidate costs more, and it bypasses 
`minCostDropPercentForScaling`; the new regression test covers the 
higher-cost-maximum case. Reviewed 9 of 9 changed files.



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