gianm commented on code in PR #19417:
URL: https://github.com/apache/druid/pull/19417#discussion_r3194227810
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/SeekableStreamSupervisorIOConfig.java:
##########
@@ -90,16 +99,19 @@ public SeekableStreamSupervisorIOConfig(
this.autoScalerConfig = autoScalerConfig;
boolean isAutoScalerAvailable = autoScalerConfig != null;
this.autoScalerEnabled = isAutoScalerAvailable &&
autoScalerConfig.getEnableTaskAutoScaler();
- if (autoScalerEnabled) {
- // Priority: taskCountStart > taskCount > taskCountMin
+ this.taskCountExplicit = taskCount != null;
+ if (taskCount != null) {
+ // Always retain taskCount when deserializing. Note: taskCountStart
takes precedence over taskCount
+ // in SeekableStreamSupervisorSpec#merge, to ensure that when a
supervisor is explicitly POSTed, taskCount
+ // is reset to taskCountStart.
+ this.taskCount = taskCount;
+ } else if (autoScalerEnabled) {
this.taskCount = Configs.valueOrDefault(
autoScalerConfig.getTaskCountStart(),
- Configs.valueOrDefault(taskCount, autoScalerConfig.getTaskCountMin())
+ autoScalerConfig.getTaskCountMin()
);
- } else if (isAutoScalerAvailable) {
- this.taskCount = Configs.valueOrDefault(taskCount,
autoScalerConfig.getTaskCountMin());
} else {
- this.taskCount = Configs.valueOrDefault(taskCount, 1);
+ this.taskCount = 1;
Review Comment:
The question here is that if `taskCount` is not provided, and an autoscaler
is provided yet disabled, what should we use for the initial `taskCount`? This
only matters on the very first submission of a supervisor, since if there's an
existing supervisor, we'll use its persisted `taskCount` (which will always be
there).
Anyway, the old code used `taskCountMin` (ignoring `taskCountStart` if
present). I thought that was strange: why use the autoscaler configs in a
slightly different way if the autoscaler is not enabled? This PR changes it to
use 1, on the grounds that if the autoscaler is submitted in a disabled state
then its configuration should not apply.
--
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]
