Skip to content
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

Merged
merged 6 commits into from
Feb 10, 2021

Conversation

xche
Copy link
Contributor

@xche xche commented Feb 7, 2021

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

  • Make sure that the change passes the CI checks.

(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:)

  • Added integration tests for end-to-end deployment with large payloads (10MB)
  • Extended integration test for recovery after broker failure

Does this pull request potentially affect one of the following parts:

If yes was chosen, please highlight the changes

  • Dependencies (does it add or upgrade a dependency): (yes / no)
  • The public API: (yes / no)
  • The schema: (yes / no / don't know)
  • The default values of configurations: (yes / no)
  • The wire protocol: (yes / no)
  • The rest endpoints: (yes / no)
  • The admin cli options: (yes / no)
  • Anything that affects deployment: (yes / no / don't know)

Documentation

  • Does this pull request introduce a new feature? (yes / no)
  • If yes, how is the feature documented? (not applicable / docs / JavaDocs / not documented)
  • If a feature is not applicable for documentation, explain why?
  • If a feature is not documented yet in this PR, please create a followup issue for adding the documentation

@jiazhai
Copy link
Member

jiazhai commented Feb 7, 2021

@xche Thanks a lot for the help on this issue. It would be great if you could also help on adding a unit test for this fix.
@freeznet Would you please also help review this PR?

@jiazhai
Copy link
Member

jiazhai commented Feb 7, 2021

/pulsarbot run-failure-checks

@freeznet
Copy link
Contributor

freeznet commented Feb 7, 2021

@xche thanks for your help on this issue, i have left some comments, PTAL, thanks.

@sijie sijie added this to the 2.8.0 milestone Feb 8, 2021
Copy link
Member

@nlu90 nlu90 left a 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();
Copy link
Member

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.

Copy link
Contributor

@eolivelli eolivelli left a 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 ?

@sijie
Copy link
Member

sijie commented Feb 9, 2021

/pulsarbot run-failure-checks

@gaoran10
Copy link
Contributor

gaoran10 commented Feb 9, 2021

/pulsarbot run-failure-checks

@codelipenghui codelipenghui merged commit 1f1b96d into apache:master Feb 10, 2021
@codelipenghui codelipenghui added the cherry-picked/branch-2.7 Archived: 2.7 is end of life label Feb 18, 2021
codelipenghui pushed a commit that referenced this pull request Feb 18, 2021
…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)
sijie pushed a commit that referenced this pull request Mar 13, 2021
### 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
fmiguelez pushed a commit to fmiguelez/pulsar that referenced this pull request Mar 16, 2021
…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
dlg99 pushed a commit to dlg99/pulsar that referenced this pull request Oct 26, 2021
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Restart all instances of a Pulsar Source failed
9 participants