FrankChen021 commented on code in PR #19922:
URL: https://github.com/apache/druid/pull/19922#discussion_r3739911387
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -464,9 +464,9 @@
"{\"taskCountMax\":10,\"taskCountMin\":1,\"minTriggerScaleActionFrequencyMillis\":900000}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.millis(900_000),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.millis(900_000),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies that a legacy-only
configuration preserves the deprecated field value during the migration window.
Removing it would reduce backward-compatibility coverage. The compatibility
test class is scoped with `@SuppressWarnings("deprecation")` and all 8 tests
pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -477,9 +477,9 @@
+ "\"minScaleUpDelay\":\"PT2M\",\"minScaleDownDelay\":\"PT15M\"}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(defaultMinTriggerMillis,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.standardMinutes(2),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.standardMinutes(15),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(defaultMinTriggerMillis,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies that a configuration
containing both the deprecated field and new directional delays preserves the
legacy value while directional values take precedence. Removing it would reduce
migration-window coverage. The compatibility test class is scoped with
`@SuppressWarnings("deprecation")` and all 8 tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -492,9 +492,9 @@
+ "\"minScaleUpDelay\":\"PT2M\",\"minScaleDownDelay\":\"PT15M\"}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.standardMinutes(2),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.standardMinutes(15),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies the mixed configuration
where only `minScaleUpDelay` is new and the deprecated field supplies the
down-delay fallback. Removing it would reduce backward-compatibility coverage.
The compatibility test class is scoped with `@SuppressWarnings("deprecation")`
and all 8 tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -506,9 +506,9 @@
+ "\"minScaleUpDelay\":\"PT2M\"}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.standardMinutes(2),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.millis(900_000),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies the mixed configuration
where only `minScaleDownDelay` is new and the deprecated field supplies the
up-delay fallback. Removing it would reduce backward-compatibility coverage.
The compatibility test class is scoped with `@SuppressWarnings("deprecation")`
and all 8 tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -520,9 +520,9 @@
+ "\"minScaleDownDelay\":\"PT15M\"}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.millis(900_000),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.standardMinutes(15),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(900_000L,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies that when the deprecated
field is omitted, its default still supplies the missing down-delay fallback
alongside a new up-delay. Removing it would reduce forward-compatibility
coverage. The compatibility test class is scoped with
`@SuppressWarnings("deprecation")` and all 8 tests pass.
##########
indexing-service/src/test/java/org/apache/druid/indexing/overlord/supervisor/autoscaler/LagBasedAutoScalerConfigTest.java:
##########
@@ -533,9 +533,9 @@
"{\"taskCountMax\":10,\"taskCountMin\":1,\"minScaleUpDelay\":\"PT2M\"}",
LagBasedAutoScalerConfig.class
);
- Assert.assertEquals(defaultMinTriggerMillis,
config.getMinTriggerScaleActionFrequencyMillis());
- Assert.assertEquals(Duration.standardMinutes(2),
config.getMinScaleUpDelay());
- Assert.assertEquals(Duration.millis(defaultMinTriggerMillis),
config.getMinScaleDownDelay());
+ Assertions.assertEquals(defaultMinTriggerMillis,
config.getMinTriggerScaleActionFrequencyMillis());
Review Comment:
Retained intentionally. This assertion verifies the deprecated field’s
default in a configuration using only the new directional delay, ensuring the
migration fallback remains stable. Removing it would reduce compatibility
coverage. The compatibility test class is scoped with
`@SuppressWarnings("deprecation")` and all 8 tests pass.
--
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]
