zhuzhurk commented on code in PR #21814:
URL: https://github.com/apache/flink/pull/21814#discussion_r1093146507
##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/DefaultSlotPoolServiceSchedulerFactoryTest.java:
##########
@@ -74,6 +76,69 @@ public void testAdaptiveSchedulerForReactiveMode() {
.isEqualTo(JobManagerOptions.SchedulerType.Adaptive);
}
+ @Test
+ public void testFallBackSchedulerWithAdaptiveTestProperty() {
Review Comment:
-> testFallBackSchedulerWithAdaptiveSchedulerTestProperty
##########
flink-runtime/src/main/java/org/apache/flink/runtime/jobmaster/DefaultSlotPoolServiceSchedulerFactory.java:
##########
@@ -232,7 +232,9 @@ private static JobManagerOptions.SchedulerType
getSchedulerType(
.getOptional(JobManagerOptions.SCHEDULER)
.orElse(
System.getProperties()
-
.containsKey("flink.tests.enable-adaptive-scheduler")
+ .containsKey(
+
"flink.tests.enable-adaptive-scheduler")
+ && jobType == JobType.STREAMING
Review Comment:
I would propose to update the fallback logics in `fromConfiguration()` too.
"Falling back to DefaultScheduler if it is a batch job" is no longer expected
after we have changed AdaptiveBatchScheduler to be the default scheduler for
batch jobs.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/DefaultSlotPoolServiceSchedulerFactoryTest.java:
##########
@@ -74,6 +76,69 @@ public void testAdaptiveSchedulerForReactiveMode() {
.isEqualTo(JobManagerOptions.SchedulerType.Adaptive);
}
+ @Test
+ public void testFallBackSchedulerWithAdaptiveTestProperty() {
+ Properties properties = System.getProperties();
+ boolean containsAdaptive =
properties.containsKey("flink.tests.enable-adaptive-scheduler");
+ if (!containsAdaptive) {
+
System.getProperties().setProperty("flink.tests.enable-adaptive-scheduler",
"true");
+ }
Review Comment:
It would be better to introduce two methods,
```
private Optional<Tuple2<String, String>> saveAdaptiveSchedulerTestProperty();
private void restoreAdaptiveSchedulerTestProperties(Optional<Tuple2<String,
String>> savedProperty);
```
And always do save at the beginning and do restore in the end.
##########
flink-runtime/src/test/java/org/apache/flink/runtime/jobmaster/DefaultSlotPoolServiceSchedulerFactoryTest.java:
##########
@@ -74,6 +76,69 @@ public void testAdaptiveSchedulerForReactiveMode() {
.isEqualTo(JobManagerOptions.SchedulerType.Adaptive);
}
+ @Test
+ public void testFallBackSchedulerWithAdaptiveTestProperty() {
+ Properties properties = System.getProperties();
+ boolean containsAdaptive =
properties.containsKey("flink.tests.enable-adaptive-scheduler");
+ if (!containsAdaptive) {
+
System.getProperties().setProperty("flink.tests.enable-adaptive-scheduler",
"true");
+ }
+
+ DefaultSlotPoolServiceSchedulerFactory
defaultSlotPoolServiceSchedulerFactory =
+ DefaultSlotPoolServiceSchedulerFactory.fromConfiguration(
+ new Configuration(), JobType.BATCH, true);
+
+
assertThat(defaultSlotPoolServiceSchedulerFactory.getSchedulerNGFactory())
+ .isInstanceOf(AdaptiveBatchSchedulerFactory.class);
+ assertThat(defaultSlotPoolServiceSchedulerFactory.getSchedulerType())
+ .isEqualTo(JobManagerOptions.SchedulerType.AdaptiveBatch);
+
+ defaultSlotPoolServiceSchedulerFactory =
+ DefaultSlotPoolServiceSchedulerFactory.fromConfiguration(
+ new Configuration(), JobType.STREAMING, false);
+
+
assertThat(defaultSlotPoolServiceSchedulerFactory.getSchedulerNGFactory())
+ .isInstanceOf(AdaptiveSchedulerFactory.class);
+ assertThat(defaultSlotPoolServiceSchedulerFactory.getSchedulerType())
+ .isEqualTo(JobManagerOptions.SchedulerType.Adaptive);
+
+ if (!containsAdaptive) {
+ properties.remove("flink.tests.enable-adaptive-scheduler");
+ }
+ }
+
+ @Test
+ public void testFallBackSchedulerWithoutAdaptiveTestProperty() {
Review Comment:
-> testFallBackSchedulerWithoutAdaptiveSchedulerTestProperty
--
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]
