You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@flink.apache.org by GitBox <gi...@apache.org> on 2022/04/18 10:19:07 UTC
[GitHub] [flink-kubernetes-operator] spoon-lz opened a new pull request, #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
spoon-lz opened a new pull request, #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171
In the "FlinkService#stopSessionCluster" method, if 'deleteHaData=true', the 'FlinkUtils#waitForClusterShutdown' method will be called twice.
So we should only execute 'FlinkUtils#waitForClusterShutdown' again with 'deleteHaData=false',This will reduce two visits to the kubernetes cluster
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] spoon-lz commented on a diff in pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
spoon-lz commented on code in PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#discussion_r852615405
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java:
##########
@@ -225,7 +225,9 @@ public Optional<String> cancelJob(
public void stopSessionCluster(
FlinkDeployment deployment, Configuration conf, boolean deleteHaData) {
FlinkUtils.deleteCluster(deployment, kubernetesClient, deleteHaData);
- FlinkUtils.waitForClusterShutdown(kubernetesClient, conf);
+ if (!deleteHaData) {
Review Comment:
FlinkService#stopSessionCluster has been removed
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] wangyang0918 merged pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
wangyang0918 merged PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171
--
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: issues-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 diff in pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on code in PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#discussion_r852977099
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java:
##########
@@ -401,12 +400,6 @@ public Optional<String> cancelSessionJob(
return savepointOpt;
}
- public void stopSessionCluster(
Review Comment:
After you rebase the latest master, I am afraid that we still need the `stopSessionCluster`, which simply call the `FlinkUtils.deleteCluster` in the production code. And we could still keep the testing implementation.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] spoon-lz commented on pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
spoon-lz commented on PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#issuecomment-1102475017
@wangyang0918 Rebase is complete
--
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: issues-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 #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#issuecomment-1102148081
@spoon-lz Could you please rebase the latest main branch and resolve the conflicts.
--
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: issues-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 diff in pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
wangyang0918 commented on code in PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#discussion_r852551483
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java:
##########
@@ -225,7 +225,9 @@ public Optional<String> cancelJob(
public void stopSessionCluster(
FlinkDeployment deployment, Configuration conf, boolean deleteHaData) {
FlinkUtils.deleteCluster(deployment, kubernetesClient, deleteHaData);
- FlinkUtils.waitForClusterShutdown(kubernetesClient, conf);
+ if (!deleteHaData) {
Review Comment:
Could we simply remove the `FlinkUtils.waitForClusterShutdown(kubernetesClient, conf)` here and call it directly in the `SessionReconciler#upgradeSessionCluster`?
Further more, we may do not need the `FlinkService#stopSessionCluster` and use `FlinkUtils.deleteCluster` instead.
--
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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [flink-kubernetes-operator] spoon-lz commented on a diff in pull request #171: [FLINK-27289]Do some optimizations for "FlinkService#stopSessionCluster"
Posted by GitBox <gi...@apache.org>.
spoon-lz commented on code in PR #171:
URL: https://github.com/apache/flink-kubernetes-operator/pull/171#discussion_r853001639
##########
flink-kubernetes-operator/src/main/java/org/apache/flink/kubernetes/operator/service/FlinkService.java:
##########
@@ -401,12 +400,6 @@ public Optional<String> cancelSessionJob(
return savepointOpt;
}
- public void stopSessionCluster(
Review Comment:
`stopSessionCluster` is back 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: issues-unsubscribe@flink.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org