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]