GJL commented on a change in pull request #8912: [FLINK-12709] [runtime] Implement new generation restart strategy loader which also respects legacy… URL: https://github.com/apache/flink/pull/8912#discussion_r328023089
########## File path: flink-runtime/src/main/java/org/apache/flink/runtime/executiongraph/failover/flip1/RestartBackoffTimeStrategyFactoryLoader.java ########## @@ -0,0 +1,135 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package org.apache.flink.runtime.executiongraph.failover.flip1; + +import org.apache.flink.api.common.restartstrategy.RestartStrategies; +import org.apache.flink.configuration.Configuration; +import org.apache.flink.configuration.RestartStrategyOptions; +import org.apache.flink.runtime.executiongraph.restart.NoOrFixedIfCheckpointingEnabledRestartStrategyFactory; +import org.apache.flink.runtime.executiongraph.restart.RestartStrategy; + +import java.util.Optional; + +import static org.apache.flink.util.Preconditions.checkNotNull; + +/** + * A utility class to load {@link RestartBackoffTimeStrategy.Factory} from the configuration. + * It respects the configs for the legacy {@link RestartStrategy}. + */ +public final class RestartBackoffTimeStrategyFactoryLoader { + + private RestartBackoffTimeStrategyFactoryLoader() { + } + + /** + * Creates {@link RestartBackoffTimeStrategy.Factory} from the given configuration. + * + * <p>The strategy factory is decided in order as follows: + * <ol> + * <li>Strategy set within job graph, i.e. {@link RestartStrategies.RestartStrategyConfiguration}, + * unless the config is {@link RestartStrategies.FallbackRestartStrategyConfiguration}.</li> + * <li>Strategy set in the cluster(server-side) config (flink-conf.yaml), + * unless the strategy is not specified</li> + * <li>{@link FixedDelayRestartBackoffTimeStrategy.FixedDelayRestartBackoffTimeStrategyFactory} if + * checkpointing is enabled. Otherwise {@link NoRestartBackoffTimeStrategy.NoRestartBackoffTimeStrategyFactory}</li> + * </ol> + * + * @param jobRestartStrategyConfiguration restart configuration given within the job graph + * @param clusterConfiguration cluster(server-side) configuration + * @param isCheckpointingEnabled if checkpointing is enabled for the job + * @return new version restart strategy factory + */ + public static RestartBackoffTimeStrategy.Factory createRestartStrategyFactory( + final RestartStrategies.RestartStrategyConfiguration jobRestartStrategyConfiguration, + final Configuration clusterConfiguration, + final boolean isCheckpointingEnabled) { + + checkNotNull(jobRestartStrategyConfiguration); + checkNotNull(clusterConfiguration); + + return getJobRestartStrategyFactory(jobRestartStrategyConfiguration) + .orElse(getClusterRestartStrategyFactory(clusterConfiguration) + .orElse(getDefaultRestartStrategyFactory(isCheckpointingEnabled))); + } + + private static Optional<RestartBackoffTimeStrategy.Factory> getJobRestartStrategyFactory( + final RestartStrategies.RestartStrategyConfiguration restartStrategyConfiguration) { + + if (restartStrategyConfiguration instanceof RestartStrategies.NoRestartStrategyConfiguration) { + return Optional.of(NoRestartBackoffTimeStrategy.NoRestartBackoffTimeStrategyFactory.INSTANCE); + } else if (restartStrategyConfiguration instanceof RestartStrategies.FixedDelayRestartStrategyConfiguration) { + final RestartStrategies.FixedDelayRestartStrategyConfiguration fixedDelayConfig = Review comment: I think this branch and the one before is not tested. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] With regards, Apache Git Services
