maytasm commented on a change in pull request #11440:
URL: https://github.com/apache/druid/pull/11440#discussion_r669344486
##########
File path:
extensions-contrib/gce-extensions/src/main/java/org/apache/druid/indexing/overlord/autoscaling/gce/GceAutoScaler.java
##########
@@ -80,8 +80,8 @@ public GceAutoScaler(
@JsonProperty("envConfig") GceEnvironmentConfig envConfig
)
{
- Preconditions.checkArgument(minNumWorkers > 0,
- "minNumWorkers must be greater than 0");
+ Preconditions.checkArgument(minNumWorkers >= 0,
Review comment:
That's is only if you are using
PendingTaskBasedWorkerProvisioningStrategy. You can have minNumWorkers = 0 and
the new config not set if you use SimpleWorkerProvisioningStrategy. I think it
makes more sense to have the check done in the
PendingTaskBasedWorkerProvisioningStrategy, instead of having a check in every
AutoScaler (EC2, GCE, etc) to check if the ProvisioningStategy ==
PendingTaskBasedWorkerProvisioningStrategy and minNumWorkers = 0 and the new
config is not set. However, this means that the failure would only show when
the auto scaler cycle runs.
--
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]
