You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@streampark.apache.org by "minchowang (via GitHub)" <gi...@apache.org> on 2023/12/20 13:56:15 UTC
[PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
minchowang opened a new pull request, #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416
<!--
Thank you for contributing to StreamPark! Please make sure that your code changes
are covered with tests. And in case of new features or big changes
remember to adjust the documentation.
## Contribution Checklist
- If this is your first time, please read our contributor guidelines: [Submit Code](https://streampark.apache.org/community/submit_guide/submit_code).
- Make sure that the pull request corresponds to a [GITHUB issue](https://github.com/apache/incubator-streampark/issues).
- Name the pull request in the form "[Feature] Title of the pull request", where *Feature* can be replaced by `Hotfix`, `Bug`, etc.
- Fill out the template below to describe the changes contributed by the pull request. That will give reviewers the context they need to do the review.
- If the PR is unfinished, add `[WIP]` in your PR title, e.g., `[WIP][Feature] Title of the pull request`.
-->
## What changes were proposed in this pull request
Issue Number: close #3388
<!--(For example: This pull request proposed to add checkstyle plugin).-->
## Brief change log
<!--*(for example:)*
- *Add maven-checkstyle-plugin to root pom.xml*
-->
## Verifying this change
<!--*(Please pick either of the following options)*-->
This change is a trivial rework / code cleanup without any test coverage.
<!--*(example:)*
- *Added integration tests for end-to-end.*
- *Added *Test to verify the change.*
- *Manually verified the change by testing locally.* -->
## Does this pull request potentially affect one of the following parts
- Dependencies (does it add or upgrade a dependency): no
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "Al-assad (via GitHub)" <gi...@apache.org>.
Al-assad merged PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "Al-assad (via GitHub)" <gi...@apache.org>.
Al-assad commented on code in PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416#discussion_r1433415681
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/KubernetesRetriever.scala:
##########
@@ -41,11 +41,11 @@ import scala.util.{Failure, Success, Try}
object KubernetesRetriever extends Logger {
// see org.apache.flink.client.cli.ClientOptions.CLIENT_TIMEOUT}
- val FLINK_CLIENT_TIMEOUT_SEC = 30L
+ val FLINK_CLIENT_TIMEOUT_SEC = 60L
// see org.apache.flink.configuration.RestOptions.AWAIT_LEADER_TIMEOUT
- val FLINK_REST_AWAIT_TIMEOUT_SEC = 10L
+ val FLINK_REST_AWAIT_TIMEOUT_SEC = 30L
// see org.apache.flink.configuration.RestOptions.RETRY_MAX_ATTEMPTS
- val FLINK_REST_RETRY_MAX_ATTEMPTS = 2
+ val FLINK_REST_RETRY_MAX_ATTEMPTS = 30
Review Comment:
I agree with you that this part affects flink task deployment and state tracking, and the state tracking part has its own additional timeout control mechanism, and increasing the retry parameter does alleviate the timeout tolerance of the submitted job.
So, LGTM
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "Al-assad (via GitHub)" <gi...@apache.org>.
Al-assad commented on code in PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416#discussion_r1432843308
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/KubernetesRetriever.scala:
##########
@@ -41,11 +41,11 @@ import scala.util.{Failure, Success, Try}
object KubernetesRetriever extends Logger {
// see org.apache.flink.client.cli.ClientOptions.CLIENT_TIMEOUT}
- val FLINK_CLIENT_TIMEOUT_SEC = 30L
+ val FLINK_CLIENT_TIMEOUT_SEC = 60L
// see org.apache.flink.configuration.RestOptions.AWAIT_LEADER_TIMEOUT
- val FLINK_REST_AWAIT_TIMEOUT_SEC = 10L
+ val FLINK_REST_AWAIT_TIMEOUT_SEC = 30L
// see org.apache.flink.configuration.RestOptions.RETRY_MAX_ATTEMPTS
- val FLINK_REST_RETRY_MAX_ATTEMPTS = 2
+ val FLINK_REST_RETRY_MAX_ATTEMPTS = 30
Review Comment:
I think it would be better to change it to 20, the default value of RestOptions.RETRY_MAX_ATTEMPTS.
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "caicancai (via GitHub)" <gi...@apache.org>.
caicancai commented on PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416#issuecomment-1864634560
cc @Al-assad
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "minchowang (via GitHub)" <gi...@apache.org>.
minchowang commented on PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416#issuecomment-1864529542
https://github.com/apache/flink/blob/aa5766e257a8b40e15d08eafa1e005837694772b/flink-core/src/main/java/org/apache/flink/configuration/RestOptions.java#L128C1-L134C60
In the `RestOptions.java`, RETRY_MAX_ATTEMPTS default value is 20, It is more than streampark RETRY_MAX_ATTEMPTS default value `2`, it might cause streampark show submit failed, but already submit to k8s.
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [Fix] Fix submit to k8s retry and timeout too less. [incubator-streampark]
Posted by "minchowang (via GitHub)" <gi...@apache.org>.
minchowang commented on code in PR #3416:
URL: https://github.com/apache/incubator-streampark/pull/3416#discussion_r1433384144
##########
streampark-flink/streampark-flink-kubernetes/src/main/scala/org/apache/streampark/flink/kubernetes/KubernetesRetriever.scala:
##########
@@ -41,11 +41,11 @@ import scala.util.{Failure, Success, Try}
object KubernetesRetriever extends Logger {
// see org.apache.flink.client.cli.ClientOptions.CLIENT_TIMEOUT}
- val FLINK_CLIENT_TIMEOUT_SEC = 30L
+ val FLINK_CLIENT_TIMEOUT_SEC = 60L
// see org.apache.flink.configuration.RestOptions.AWAIT_LEADER_TIMEOUT
- val FLINK_REST_AWAIT_TIMEOUT_SEC = 10L
+ val FLINK_REST_AWAIT_TIMEOUT_SEC = 30L
// see org.apache.flink.configuration.RestOptions.RETRY_MAX_ATTEMPTS
- val FLINK_REST_RETRY_MAX_ATTEMPTS = 2
+ val FLINK_REST_RETRY_MAX_ATTEMPTS = 30
Review Comment:
> I think it would be better to change it to 20, the default value of RestOptions.RETRY_MAX_ATTEMPTS.
I think the redundant number of times can cover unknown issues.
--
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@streampark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org