[
https://issues.apache.org/jira/browse/FLINK-40559?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
ASF GitHub Bot updated FLINK-40559:
-----------------------------------
Labels: pull-request-available (was: )
> Application mode silently ignores the --claimMode / --restoreMode
> command-line option
> -------------------------------------------------------------------------------------
>
> Key: FLINK-40559
> URL: https://issues.apache.org/jira/browse/FLINK-40559
> Project: Flink
> Issue Type: Bug
> Components: Client / Job Submission, Deployment / Kubernetes
> Affects Versions: 2.0.0, 1.18.1, 1.20.2, 2.1.0, 2.3.0, 2.2.1
> Reporter: Aaron He
> Priority: Major
> Labels: pull-request-available
>
> Flink silently discards an explicitly passed {{claimMode}} command-line
> option (or its deprecated alias {{restoreMode}}) in two independent places.
> The user receives no warning or error; the recovery claim mode falls back to
> {{NO_CLAIM}}.
> Verified on {{master}} at
> [b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0|https://github.com/apache/flink/commit/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0].
> h2. References
> * [Application-mode parser
> options|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneApplicationClusterConfigurationParserFactory.java#L75-L86]
> * [CommandLineParser using stopAtNonOption =
> true|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-runtime/src/main/java/org/apache/flink/runtime/entrypoint/parser/CommandLineParser.java#L40-L50]
> * [createSavepointRestoreSettings() savepoint-path
> gate|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-clients/src/main/java/org/apache/flink/client/cli/CliFrontendParser.java#L654-L686]
> * [SavepointRestoreSettings.toConfiguration() /
> fromConfiguration()|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-runtime/src/main/java/org/apache/flink/runtime/jobgraph/SavepointRestoreSettings.java#L195-L229]
> * [Application-mode direct configuration
> propagation|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-container/src/main/java/org/apache/flink/container/entrypoint/StandaloneApplicationClusterEntryPoint.java#L115-L121]
> * [ProgramOptions parses and serializes
> SavepointRestoreSettings|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-clients/src/main/java/org/apache/flink/client/cli/ProgramOptions.java#L114-L118]
> *
> [ProgramOptions.applyToConfiguration()|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-clients/src/main/java/org/apache/flink/client/cli/ProgramOptions.java#L175-L189]
> * [ExecutionConfigAccessor reconstructs
> SavepointRestoreSettings|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-clients/src/main/java/org/apache/flink/client/cli/ExecutionConfigAccessor.java#L57-L68]
> *
> [ExecutionConfigAccessor.getSavepointRestoreSettings()|https://github.com/apache/flink/blob/b53ea1b1c4d5ab0ec882559a4a050d82ae5740e0/flink-clients/src/main/java/org/apache/flink/client/cli/ExecutionConfigAccessor.java#L93-L95]
> * [FLINK-39673|https://issues.apache.org/jira/browse/FLINK-39673] and its
> merged fix, [apache/flink#28295|https://github.com/apache/flink/pull/28295].
> The PR title says {{FLINK-36973}}, but its body correctly links
> {{FLINK-39673}}.
> * [Flink code contribution
> process|https://flink.apache.org/how-to-contribute/contribute-code/]
> * [Flink pull-request quality
> guide|https://flink.apache.org/how-to-contribute/code-style-and-quality-pull-requests/]
> h2. Defect A: the option is never registered in application mode
> {{StandaloneApplicationClusterConfigurationParserFactory.getOptions()}}
> registers {{SAVEPOINT_PATH_OPTION}} and
> {{SAVEPOINT_ALLOW_NON_RESTORED_OPTION}}, but not {{SAVEPOINT_CLAIM_MODE}} or
> {{SAVEPOINT_RESTORE_MODE}}:
> {noformat}
> options.addOption(CliFrontendParser.SAVEPOINT_PATH_OPTION);
> options.addOption(CliFrontendParser.SAVEPOINT_ALLOW_NON_RESTORED_OPTION);
> // no SAVEPOINT_CLAIM_MODE / SAVEPOINT_RESTORE_MODE
> {noformat}
> {{CommandLineParser}} calls {{parser.parse(options, args, true)}} with
> {{stopAtNonOption = true}}, so an unrecognized {{claimMode}} option does not
> raise an error. It terminates option parsing and the remainder is swallowed
> into {{getArgs()}}. The claim mode is therefore unreachable in application
> mode regardless of how it is passed.
> h2. Defect B: the parse is gated on the fromSavepoint option
> {{CliFrontendParser.createSavepointRestoreSettings()}} reads the claim mode
> only inside the savepoint-path branch:
> {noformat}
> if (commandLine.hasOption(SAVEPOINT_PATH_OPTION.getOpt())) {
> ...
> if (commandLine.hasOption(SAVEPOINT_CLAIM_MODE)) { ... }
> return SavepointRestoreSettings.forPath(
> savepointPath, allowNonRestoredState, recoveryClaimMode);
> } else {
> return SavepointRestoreSettings.none(); // claim mode discarded
> }
> {noformat}
> When the {{claimMode}} option is passed without the {{fromSavepoint}} option,
> it is parsed and then dropped. This is the normal shape for an HA-recovery
> restart, where the JobManager resolves its checkpoint from HA storage rather
> than from a command-line savepoint path.
> The two defects are independent: fixing A alone still leaves B.
> h2. How to reproduce
> Add these tests to
> {{StandaloneApplicationClusterConfigurationParserFactoryTest}}:
> {noformat}
> @Test
> void testClaimModeWithSavepoint() throws FlinkParseException {
> final String restorePath = "s3://test/savepoint";
> final String[] args = {
> "-c", confDirPath, "-j", JOB_CLASS_NAME,
> "--fromSavepoint", restorePath,
> "--claimMode", RecoveryClaimMode.CLAIM.name()
> };
> final SavepointRestoreSettings settings =
> commandLineParser.parse(args).getSavepointRestoreSettings();
> assertThat(settings.getRestorePath()).isEqualTo(restorePath);
>
> assertThat(settings.getRecoveryClaimMode()).isEqualTo(RecoveryClaimMode.CLAIM);
> }
> @Test
> void testClaimModeWithoutSavepoint() throws FlinkParseException {
> final String[] args = {
> "-c", confDirPath, "-j", JOB_CLASS_NAME,
> "--claimMode", RecoveryClaimMode.CLAIM.name()
> };
> final SavepointRestoreSettings settings =
> commandLineParser.parse(args).getSavepointRestoreSettings();
> assertThat(settings.restoreSavepoint()).isFalse();
>
> assertThat(settings.getRecoveryClaimMode()).isEqualTo(RecoveryClaimMode.CLAIM);
> }
> {noformat}
> Run:
> {noformat}
> ./mvnw -pl flink-container
> -Dtest='StandaloneApplicationClusterConfigurationParserFactoryTest#testClaimModeWithSavepoint+testClaimModeWithoutSavepoint'
> test
> {noformat}
> *Observed on unmodified {{master}}:* both tests fail because Defect A masks
> Defect B.
> {noformat}
> [ERROR] Tests run: 2, Failures: 2, Errors: 0, Skipped: 0
> [ERROR] ...testClaimModeWithSavepoint
> expected: CLAIM
> but was: NO_CLAIM
> [ERROR] ...testClaimModeWithoutSavepoint
> expected: CLAIM
> but was: NO_CLAIM
> {noformat}
> To isolate Defect B, temporarily add {{SAVEPOINT_CLAIM_MODE}} and
> {{SAVEPOINT_RESTORE_MODE}} to {{getOptions()}} and rerun:
> {noformat}
> [ERROR] Tests run: 2, Failures: 1, Errors: 0, Skipped: 0
> [ERROR] ...testClaimModeWithoutSavepoint
> expected: CLAIM
> but was: NO_CLAIM
> {noformat}
> {{testClaimModeWithSavepoint}} now passes while
> {{testClaimModeWithoutSavepoint}} still fails, confirming that the two
> defects are independent.
> h2. Relationship to FLINK-39673
> [FLINK-39673|https://issues.apache.org/jira/browse/FLINK-39673] / [PR
> #28295|https://github.com/apache/flink/pull/28295] made
> {{SavepointRestoreSettings.toConfiguration()}} skip options that were not
> explicitly set, and {{none()}} is now {{(null, null, null)}}. As a result, an
> unset claim mode is no longer written to the {{Configuration}}, so a value
> supplied through {{flink-conf.yaml}} / {{flinkConfiguration}} survives
> correctly.
> This ticket is therefore not about configuration being overwritten. The
> remaining defect is narrower: an explicitly passed {{claimMode}} command-line
> option is silently ignored, so the option is non-functional in application
> mode.
> h2. Proposed fix
> Two small, independently reviewable commits:
> # *{{flink-container}}:* Register {{SAVEPOINT_CLAIM_MODE}} and
> {{SAVEPOINT_RESTORE_MODE}} in
> {{StandaloneApplicationClusterConfigurationParserFactory.getOptions()}}.
> # *{{flink-clients}} / {{flink-runtime}}:* Restructure
> {{createSavepointRestoreSettings()}} to branch on option presence rather than
> on the presence of a savepoint path, and add a factory on
> {{SavepointRestoreSettings}} that carries a claim mode with no restore path.
> The second change must preserve the post-FLINK-39673 "explicitly set"
> semantics. {{allowNonRestoredState}} must remain {{null}} when the flag is
> absent, so the new code path does not begin writing
> {{execution.state-recovery.ignore-unclaimed-state}} into the
> {{Configuration}} where it previously wrote nothing.
> Behavior is unchanged for jobs that do not pass the {{claimMode}} option.
> h2. Related but out of scope
> There is a separate round-trip asymmetry in session-mode job submission:
> # {{ProgramOptions}} calls {{createSavepointRestoreSettings()}} and receives
> a pathless settings object containing the explicitly requested claim mode.
> # {{ProgramOptions.applyToConfiguration()}} writes that mode to
> {{StateRecoveryOptions.RESTORE_MODE}}, but correctly does not write
> {{StateRecoveryOptions.SAVEPOINT_PATH}}.
> # {{ExecutionConfigAccessor.getSavepointRestoreSettings()}} then reconstructs
> the object by calling {{SavepointRestoreSettings.fromConfiguration()}}.
> # {{fromConfiguration()}} checks {{SAVEPOINT_PATH}} first and immediately
> returns {{none()}} when the path is absent, before reading {{RESTORE_MODE}}.
> The explicit claim mode is therefore lost on this round trip.
> This can be observed in the regular {{flink run}} path when a claim mode is
> supplied without a savepoint path:
> {noformat}
> bin/flink run --claimMode CLAIM job.jar
> {noformat}
> It is different from the application-mode startup path covered by this
> ticket: {{StandaloneApplicationClusterConfigurationParserFactory}} retains
> the parsed {{SavepointRestoreSettings}} object, and
> {{StandaloneApplicationClusterEntryPoint.loadConfigurationFromClusterConfig()}}
> passes it directly to {{toConfiguration()}}. Application mode never
> reconstructs it through {{fromConfiguration()}}.
> Changing {{fromConfiguration()}} would affect the general session-mode CLI
> and other configuration consumers, not just the application-mode JobManager
> entrypoint. Also, the CLI option is documented in terms of restoring from a
> given savepoint, so the intended semantics of a pathless claim mode for a
> regular session-mode submission should be agreed separately. A follow-up
> could preserve the mode by returning {{forRecoveryClaimMode(...)}} when
> {{RESTORE_MODE}} is explicitly present but {{SAVEPOINT_PATH}} is absent.
> This ticket therefore limits the change to application-mode startup. I am
> happy to include the round-trip change here or file a follow-up if reviewers
> consider pathless claim mode part of the general CLI contract.
> I have the application-mode fix and test coverage prepared and am happy to
> take this ticket.
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
