You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@flink.apache.org by GitBox <gi...@apache.org> on 2022/03/08 19:45:30 UTC
[GitHub] [flink-kubernetes-operator] gyfora opened a new pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
gyfora opened a new pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48
The main goal of this PR is to allow upgrading/suspending clusters even if they are still deploying / failing under certain conditions:
- Session cluster
- Job cluster with stateless or last-state upgrade modes
To achieve this, the PR simplifies the observe -> reconcile flow by removing the sketchy isReadyToReconcile logic in between and moving the contained logic to the right places in the reconcilers.
I have also added test cases to validate the upgrade behaviour in cases where the cluster is not ready/running yet for all cluster types.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822645293
##########
File path: flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java
##########
@@ -295,6 +302,113 @@ public void verifyStatelessUpgrade() {
assertEquals(null, jobs.get(0).f0);
}
+ @Test
+ public void testUpgradeNotReadyCluster() {
+ testUpgradeNotReadyCluster(TestUtils.buildSessionCluster(), true);
+
+ FlinkDeployment appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.STATELESS);
+ testUpgradeNotReadyCluster(appCluster, true);
+
+ appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.LAST_STATE);
+ testUpgradeNotReadyCluster(appCluster, true);
+
+ appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.SAVEPOINT);
+ testUpgradeNotReadyCluster(appCluster, false);
+ }
+
+ public void testUpgradeNotReadyCluster(FlinkDeployment appCluster, boolean allowUpgrade) {
+ mockServer
Review comment:
I tried this now and this would work for the newly added tests as they don't test anything, but for the events it doesn't seem to work for some reason. The list doesn't give back events created earlier in the controller.
I will open a jira ticket to try to solve this
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822568654
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -76,14 +78,15 @@ public JobReconciler(
IngressUtils.updateIngressRules(
flinkApp, effectiveConfig, operatorNamespace, kubernetesClient, false);
ReconciliationUtils.updateForSpecReconciliationSuccess(flinkApp);
- return JobManagerDeploymentStatus.DEPLOYING.toUpdateControl(
- flinkApp, operatorConfiguration);
+ return;
+ }
+
+ if (SavepointUtils.savepointInProgress(flinkApp)) {
+ return;
Review comment:
It will be great if we have a log here. Then users will know clearly the reconciliation is delayed because of savepoint is in progress.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822485643
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -54,10 +56,19 @@ public Observer(FlinkService flinkService, FlinkOperatorConfiguration operatorCo
this.operatorConfiguration = operatorConfiguration;
}
- public boolean observe(
- FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+ public void observe(FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+
observeJmDeployment(flinkApp, context, effectiveConfig);
- return isReadyToReconcile(flinkApp, effectiveConfig);
+ JobManagerDeploymentStatus deploymentStatus =
+ flinkApp.getStatus().getJobManagerDeploymentStatus();
+
+ if (deploymentStatus == JobManagerDeploymentStatus.READY
+ && flinkApp.getSpec().getJob() != null) {
Review comment:
Thanks for the ticket, I improved this check to be more descriptive
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
morhidi commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822410633
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -54,10 +56,19 @@ public Observer(FlinkService flinkService, FlinkOperatorConfiguration operatorCo
this.operatorConfiguration = operatorConfiguration;
}
- public boolean observe(
- FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+ public void observe(FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+
observeJmDeployment(flinkApp, context, effectiveConfig);
- return isReadyToReconcile(flinkApp, effectiveConfig);
+ JobManagerDeploymentStatus deploymentStatus =
+ flinkApp.getStatus().getJobManagerDeploymentStatus();
+
+ if (deploymentStatus == JobManagerDeploymentStatus.READY
+ && flinkApp.getSpec().getJob() != null) {
Review comment:
We can consider introducing a JobObserver/SessionObserver pair too similar to JobReconciler/SessionReconciler pair.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822624101
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -76,14 +78,15 @@ public JobReconciler(
IngressUtils.updateIngressRules(
flinkApp, effectiveConfig, operatorNamespace, kubernetesClient, false);
ReconciliationUtils.updateForSpecReconciliationSuccess(flinkApp);
- return JobManagerDeploymentStatus.DEPLOYING.toUpdateControl(
- flinkApp, operatorConfiguration);
+ return;
+ }
+
+ if (SavepointUtils.savepointInProgress(flinkApp)) {
+ return;
Review comment:
Good idea, adding log.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
morhidi commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822410633
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -54,10 +56,19 @@ public Observer(FlinkService flinkService, FlinkOperatorConfiguration operatorCo
this.operatorConfiguration = operatorConfiguration;
}
- public boolean observe(
- FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+ public void observe(FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+
observeJmDeployment(flinkApp, context, effectiveConfig);
- return isReadyToReconcile(flinkApp, effectiveConfig);
+ JobManagerDeploymentStatus deploymentStatus =
+ flinkApp.getStatus().getJobManagerDeploymentStatus();
+
+ if (deploymentStatus == JobManagerDeploymentStatus.READY
+ && flinkApp.getSpec().getJob() != null) {
Review comment:
We can consider introducing a JobObserver/SessionObserver pair too similarly to JobReconciler/SessionReconciler.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#issuecomment-1062140615
cc @tweise @wangyang0918
Thomas, I believe this is orthogonal to your error handling changes and will integrate nicely with it.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] wangyang0918 commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822567083
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -169,16 +189,24 @@ private void printCancelLogs(UpgradeMode upgradeMode, String name) {
}
}
- private Optional<String> cancelJob(
+ private Optional<String> suspendJob(
FlinkDeployment flinkApp, UpgradeMode upgradeMode, Configuration effectiveConfig)
throws Exception {
- Optional<String> savepointOpt =
- flinkService.cancelJob(
- JobID.fromHexString(flinkApp.getStatus().getJobStatus().getJobId()),
- upgradeMode,
- effectiveConfig);
+
+ Optional<String> savepointOpt = Optional.empty();
+ if (upgradeMode == UpgradeMode.STATELESS) {
+ shutdown(flinkApp, effectiveConfig);
+ } else {
+ String jobIdString = flinkApp.getStatus().getJobStatus().getJobId();
+ savepointOpt =
+ flinkService.cancelJob(
+ jobIdString != null ? JobID.fromHexString(jobIdString) : null,
Review comment:
It is not very good that the `jobId` could be `null` here. How about the following changes?
```
Optional<String> savepointOpt = Optional.empty();
// The job should be running if UpgradeMode is savepoint
if (!isJobRunning(flinkApp)) {
FlinkUtils.deleteCluster(flinkApp, kubernetesClient, true);
return savepointOpt;
}
final String jobIdString = flinkApp.getStatus().getJobStatus().getJobId();
savepointOpt =
flinkService.cancelJob(
JobID.fromHexString(jobIdString), upgradeMode, effectiveConfig);
```
##########
File path: flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/TestUtils.java
##########
@@ -72,19 +75,26 @@ public static FlinkDeployment buildApplicationCluster() {
JobSpec.builder()
.jarURI(SAMPLE_JAR)
.parallelism(1)
+ .upgradeMode(UpgradeMode.STATELESS)
.state(JobState.RUNNING)
.build());
return deployment;
}
public static FlinkDeploymentSpec getTestFlinkDeploymentSpec() {
+ Map<String, String> conf = new HashMap<>();
+ conf.put(TaskManagerOptions.NUM_TASK_SLOTS.key(), "2");
+ conf.put(
Review comment:
We could use `HighAvailabilityOptions.HA_MODE` and `KubernetesHaServicesFactory.class.getCanonicalName()` here.
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -76,14 +78,15 @@ public JobReconciler(
IngressUtils.updateIngressRules(
flinkApp, effectiveConfig, operatorNamespace, kubernetesClient, false);
ReconciliationUtils.updateForSpecReconciliationSuccess(flinkApp);
- return JobManagerDeploymentStatus.DEPLOYING.toUpdateControl(
- flinkApp, operatorConfiguration);
+ return;
+ }
+
+ if (SavepointUtils.savepointInProgress(flinkApp)) {
+ return;
Review comment:
It will be great if we have a log here. Then users will know clearly the reconciliation is delayed because of savepoint in progress.
##########
File path: flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/controller/FlinkDeploymentControllerTest.java
##########
@@ -295,6 +302,113 @@ public void verifyStatelessUpgrade() {
assertEquals(null, jobs.get(0).f0);
}
+ @Test
+ public void testUpgradeNotReadyCluster() {
+ testUpgradeNotReadyCluster(TestUtils.buildSessionCluster(), true);
+
+ FlinkDeployment appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.STATELESS);
+ testUpgradeNotReadyCluster(appCluster, true);
+
+ appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.LAST_STATE);
+ testUpgradeNotReadyCluster(appCluster, true);
+
+ appCluster = TestUtils.buildApplicationCluster();
+ appCluster.getSpec().getJob().setUpgradeMode(UpgradeMode.SAVEPOINT);
+ testUpgradeNotReadyCluster(appCluster, false);
+ }
+
+ public void testUpgradeNotReadyCluster(FlinkDeployment appCluster, boolean allowUpgrade) {
+ mockServer
Review comment:
I am not sure whether we could use `@EnableKubernetesMockClient(crud = true)` for this test class just like `FlinkServiceTest`.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora merged pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora merged pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] SteNicholas commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822172401
##########
File path: flink-kubernetes-operator/src/test/java/org/apache/flink/kubernetes/operator/observer/ObserverTest.java
##########
@@ -103,41 +99,43 @@ public void observeApplicationCluster() {
flinkService.setPortReady(false);
// Port not ready
- assertFalse(observer.observe(deployment, readyContext, conf));
+ observer.observe(deployment, readyContext, conf);
assertEquals(
JobManagerDeploymentStatus.DEPLOYING,
deployment.getStatus().getJobManagerDeploymentStatus());
- assertFalse(observer.observe(deployment, readyContext, conf));
+ observer.observe(deployment, readyContext, conf);
assertEquals(
JobManagerDeploymentStatus.DEPLOYING,
deployment.getStatus().getJobManagerDeploymentStatus());
flinkService.setPortReady(true);
// Port ready but we have to recheck once again
- assertFalse(observer.observe(deployment, readyContext, conf));
+ observer.observe(deployment, readyContext, conf);
assertEquals(
JobManagerDeploymentStatus.DEPLOYED_NOT_READY,
deployment.getStatus().getJobManagerDeploymentStatus());
// Stable ready
- assertTrue(observer.observe(deployment, readyContext, conf));
+ observer.observe(deployment, readyContext, conf);
assertEquals(
JobManagerDeploymentStatus.READY,
deployment.getStatus().getJobManagerDeploymentStatus());
+ assertEquals("RUNNING", deployment.getStatus().getJobStatus().getState());
- assertTrue(observer.observe(deployment, readyContext, conf));
+ observer.observe(deployment, readyContext, conf);
assertEquals(
JobManagerDeploymentStatus.READY,
deployment.getStatus().getJobManagerDeploymentStatus());
+ assertEquals("RUNNING", deployment.getStatus().getJobStatus().getState());
Review comment:
It's better to replace "RUNNING" with the `JobState.RUNNING.name()`.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] SteNicholas commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822170223
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -46,6 +46,8 @@
private static final Logger LOG = LoggerFactory.getLogger(Observer.class);
+ public static final String JOB_STATE_UNKNOWN = "UNKNOWN";
Review comment:
Does this need to add the enum `UNKOWN` in `JobState` and directly use this enum?
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#issuecomment-1062780180
Based on my local tests this works pretty well now. Users can now recover from stuck deployments / deployment errors for stateless/last-state jobs.
Let me know if you have any other comments.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822623850
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -169,16 +189,24 @@ private void printCancelLogs(UpgradeMode upgradeMode, String name) {
}
}
- private Optional<String> cancelJob(
+ private Optional<String> suspendJob(
FlinkDeployment flinkApp, UpgradeMode upgradeMode, Configuration effectiveConfig)
throws Exception {
- Optional<String> savepointOpt =
- flinkService.cancelJob(
- JobID.fromHexString(flinkApp.getStatus().getJobStatus().getJobId()),
- upgradeMode,
- effectiveConfig);
+
+ Optional<String> savepointOpt = Optional.empty();
+ if (upgradeMode == UpgradeMode.STATELESS) {
+ shutdown(flinkApp, effectiveConfig);
+ } else {
+ String jobIdString = flinkApp.getStatus().getJobStatus().getJobId();
+ savepointOpt =
+ flinkService.cancelJob(
+ jobIdString != null ? JobID.fromHexString(jobIdString) : null,
Review comment:
The change you propose would simply delete running clusters including HA data which would break both savepoint and last-state upgrade modes.
The problem currently is that we use the cancelJob call even in cases where the job might not be running (in last-state mode). In these cases the Job id is not used. We can improve this in the future but at least this seems to work well now.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
morhidi commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822443842
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -54,10 +56,19 @@ public Observer(FlinkService flinkService, FlinkOperatorConfiguration operatorCo
this.operatorConfiguration = operatorConfiguration;
}
- public boolean observe(
- FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+ public void observe(FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+
observeJmDeployment(flinkApp, context, effectiveConfig);
- return isReadyToReconcile(flinkApp, effectiveConfig);
+ JobManagerDeploymentStatus deploymentStatus =
+ flinkApp.getStatus().getJobManagerDeploymentStatus();
+
+ if (deploymentStatus == JobManagerDeploymentStatus.READY
+ && flinkApp.getSpec().getJob() != null) {
Review comment:
I've opened a Jira for this https://issues.apache.org/jira/browse/FLINK-26546
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] gyfora commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
gyfora commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822485189
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -46,6 +46,8 @@
private static final Logger LOG = LoggerFactory.getLogger(Observer.class);
+ public static final String JOB_STATE_UNKNOWN = "UNKNOWN";
Review comment:
No, this has nothing to do with expected job states. This means that we don't know the current state. We expected state (JobState enum) is always known.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] SteNicholas commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822170959
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/reconciler/JobReconciler.java
##########
@@ -169,16 +189,24 @@ private void printCancelLogs(UpgradeMode upgradeMode, String name) {
}
}
- private Optional<String> cancelJob(
+ private Optional<String> suspendJob(
FlinkDeployment flinkApp, UpgradeMode upgradeMode, Configuration effectiveConfig)
throws Exception {
- Optional<String> savepointOpt =
- flinkService.cancelJob(
- JobID.fromHexString(flinkApp.getStatus().getJobStatus().getJobId()),
- upgradeMode,
- effectiveConfig);
+
+ Optional<String> savepointOpt = Optional.empty();
+ if (upgradeMode == UpgradeMode.STATELESS) {
+ shutdown(flinkApp, effectiveConfig);
+ } else {
+ String jobIdString = flinkApp.getStatus().getJobStatus().getJobId();
+ savepointOpt =
+ flinkService.cancelJob(
+ jobIdString != null ? JobID.fromHexString(jobIdString) : null,
+ upgradeMode,
+ effectiveConfig);
+ }
+
JobStatus jobStatus = flinkApp.getStatus().getJobStatus();
- jobStatus.setState("suspended");
+ jobStatus.setState("SUSPENDED");
Review comment:
Could this replace "SUSPENDED" with `JobState.SUSPENDED.name()`?
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] wangyang0918 commented on pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#issuecomment-1062806920
I am getting to this PR now and will give a pass soon.
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] morhidi commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
morhidi commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822450320
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -54,10 +56,19 @@ public Observer(FlinkService flinkService, FlinkOperatorConfiguration operatorCo
this.operatorConfiguration = operatorConfiguration;
}
- public boolean observe(
- FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+ public void observe(FlinkDeployment flinkApp, Context context, Configuration effectiveConfig) {
+
observeJmDeployment(flinkApp, context, effectiveConfig);
- return isReadyToReconcile(flinkApp, effectiveConfig);
+ JobManagerDeploymentStatus deploymentStatus =
+ flinkApp.getStatus().getJobManagerDeploymentStatus();
+
+ if (deploymentStatus == JobManagerDeploymentStatus.READY
+ && flinkApp.getSpec().getJob() != null) {
Review comment:
Great job @gyfora!
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] SteNicholas commented on a change in pull request #48: [FLINK-26507] Allow session and last-state jobs to reconcile in any state
Posted by GitBox <gi...@apache.org>.
SteNicholas commented on a change in pull request #48:
URL: https://github.com/apache/flink-kubernetes-operator/pull/48#discussion_r822166722
##########
File path: flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/observer/Observer.java
##########
@@ -46,6 +46,8 @@
private static final Logger LOG = LoggerFactory.getLogger(Observer.class);
+ public static final String JOB_STATE_UNKNOWN = "UNKNOWN";
Review comment:
Does this need to introdce the enum for the job state?
--
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: commits-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org