-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Functions] Call the corresponding restart according to the componenttype. #9502 #9519
Conversation
update new
Synchronization code
/pulsarbot run-failure-checks |
...unctions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
Outdated
Show resolved
Hide resolved
...unctions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
Show resolved
Hide resolved
@xche thanks for your help on this issue, i have left some comments, PTAL, thanks. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM except the minor comment
this.functionAdmin.functions().restartFunction(tenant, namespace, functionName, | ||
assignment.getInstance().getInstanceId()); | ||
|
||
ComponentType componentType = assignment.getInstance().getFunctionMetaData().getFunctionDetails().getComponentType(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the code here and on line 422 - 429 are highly similar.
Could you please abstract a method and then reuse it for these two places.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks good overall,
can you please add a few unit tests in order to prevent regressions in the future ?
/pulsarbot run-failure-checks |
/pulsarbot run-failure-checks |
…type. #9502 (#9519) Fixes #9502 Add judgment on component type,Call the corresponding restart according to the componenttype ### Motivation Restart all instances of a Pulsar Source failed. When I call the "https://pulsar.apache.org/admin/v3/sources/{tenant}/{namespace}/{sourceName}/restart" See error "Failed to perform http post request: javax.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error Function xxxx doesn't exist". The reason for the problem is that the corresponding functionadmin is not found according to the componenttype, which leads to the restart of sink or source using the functionadmin of function (cherry picked from commit 1f1b96d)
### Motivation This PR is related to #9502 and [PR #9519](#9519). In [PR #9519 ](#9519), another contributer has fixed the problem described in #9502, but there are still some areas that can be optimized. ### Modifications - Abstract a method and then reuse some code. - Adding a unit test for the `restartFunctionInstances` method
…pache#9671) ### Motivation This PR is related to apache#9502 and [PR apache#9519](apache#9519). In [PR apache#9519 ](apache#9519), another contributer has fixed the problem described in apache#9502, but there are still some areas that can be optimized. ### Modifications - Abstract a method and then reuse some code. - Adding a unit test for the `restartFunctionInstances` method
…pache#9671) This PR is related to apache#9502 and [PR apache#9519](apache#9519). In [PR apache#9519 ](apache#9519), another contributer has fixed the problem described in apache#9502, but there are still some areas that can be optimized. - Abstract a method and then reuse some code. - Adding a unit test for the `restartFunctionInstances` method
Fixes #9502
Add judgment on component type,Call the corresponding restart according to the componenttype
Motivation
Restart all instances of a Pulsar Source failed.
When I call the "https://pulsar.apache.org/admin/v3/sources/{tenant}/{namespace}/{sourceName}/restart"
See error "Failed to perform http post request: javax.ws.rs.InternalServerErrorException: HTTP 500 Internal Server Error
Function xxxx doesn't exist".
The reason for the problem is that the corresponding functionadmin is not found according to the componenttype, which leads to the restart of sink or source using the functionadmin of function
Modifications
Describe the modifications you've done.
Verifying this change
(Please pick either of the following options)
This change is a trivial rework / code cleanup without any test coverage.
(or)
This change is already covered by existing tests, such as (please describe tests).
(or)
This change added tests and can be verified as follows:
(example:)
Does this pull request potentially affect one of the following parts:
If
yes
was chosen, please highlight the changesDocumentation