Fly-Style commented on code in PR #19417:
URL: https://github.com/apache/druid/pull/19417#discussion_r3194062626
##########
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:
Why this change? Autoscaler also may be _disabled_, but spec may not be null
(available).
Isn't it better to assign 1 only as a last resort?
AFAIR, this LOC led to some inaccurate behaviour some time ago, but do not
remember exactly what it was.
##########
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) {
Review Comment:
nit: may use `this.taskCountExplicit` for condition
--
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]
