james-kan-shopify commented on code in PR #1199:
URL:
https://github.com/apache/flink-kubernetes-operator/pull/1199#discussion_r3896571600
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/ApplicationReconciler.java:
##########
@@ -273,6 +285,24 @@ private static void
setRandomJobResultStorePath(Configuration effectiveConfig) {
}
}
+ private static void setRandomApplicationResultStorePath(Configuration
effectiveConfig) {
+ if (effectiveConfig.contains(HighAvailabilityOptions.HA_STORAGE_PATH))
{
Review Comment:
Hi! I've encountered the same bug, and have been working in a fork regarding
this. Thanks for opening up a PR here!
Suggesting to consider also setting
`application-result-store.delete-on-commit=false`,
mirroring`setRandomJobResultStorePath`.
`maybeSubmitApplicationInApplicationMode` skips submission on a clean result
entry, but per its javadoc that lookup only works when
`DELETE_ON_COMMIT=false`. Since the operator sets
`shutdown-on-application-finish=false`, a terminal app's JM lives on until the
shutdown TTL, and a restart in that window re-bootstraps it: `main()` re-runs,
and multi-job apps fail outright. FLINK-27569 one level up.
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/deployment/ApplicationReconciler.java:
##########
@@ -273,6 +285,24 @@ private static void
setRandomJobResultStorePath(Configuration effectiveConfig) {
}
}
+ private static void setRandomApplicationResultStorePath(Configuration
effectiveConfig) {
+ if (effectiveConfig.contains(HighAvailabilityOptions.HA_STORAGE_PATH))
{
+ // The application result store shares the HA storage path, which
outlives
+ // the HA metadata deleted on upgrade. Dirty entries are recovered
unkeyed,
+ // so a terminal entry left behind by a previous deployment makes
the
+ // replacement cluster skip submitting the new job.
+ // Giving each deployment a unique path makes sure that nothing
stale is
+ // recovered.
+ effectiveConfig.set(
Review Comment:
Curious, if you think it is worthwhile to gate this behind a version check?
Since this is unique to jobs post 2.3, we'd be unnecessarily creating entries
in storage. Given that this fix addresses what may be a cleanup race condition
under the hood, but isn't directly resolving the race, it may be ideal to
minimize it to 2.3 onwards?
--
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]
