gianm commented on code in PR #17900:
URL: https://github.com/apache/druid/pull/17900#discussion_r2047872048
##########
indexing-service/src/main/java/org/apache/druid/indexing/seekablestream/supervisor/autoscaler/LagBasedAutoScalerConfig.java:
##########
@@ -85,9 +87,12 @@ public LagBasedAutoScalerConfig(
throw new RuntimeException("taskCountMax or taskCountMin can't be
null!");
} else if (taskCountMax < taskCountMin) {
throw new RuntimeException("taskCountMax can't lower than
taskCountMin!");
+ } else if (taskCountStart != null && (taskCountStart > taskCountMax ||
taskCountStart < taskCountMin)) {
+ throw new RuntimeException("taskCountMin <= taskCountStart <=
taskCountMax");
}
this.taskCountMax = taskCountMax;
this.taskCountMin = taskCountMin;
+ this.taskCountStart = taskCountStart != null ? taskCountStart :
taskCountMin;
Review Comment:
IMO, it's best for an optional parameter to use a `@Nullable` field and to
have callers handle the null case. This is because it keeps the serialized JSON
cleaner: it won't include `taskCountStart` unless the user actually provided
it. Concrete suggestions:
- Change the field `private int taskCountStart` to `@Nullable private
Integer taskCountStart`
- Change this line to `this.taskCountStart = taskCountStart`
- Add `@JsonInclude(JsonInclude.Include.NON_NULL)` to `getTaskCountStart()`
so it won't show up if null
- At the call site, handle the `getTaskCountStart() == null` case by calling
`getTaskCountMin()` instead.
--
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]
