You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pulsar.apache.org by GitBox <gi...@apache.org> on 2021/02/07 10:00:42 UTC
[GitHub] [pulsar] xche opened a new pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
xche opened a new pull request #9519:
URL: https://github.com/apache/pulsar/pull/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
### 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
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#issuecomment-775755345
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on a change in pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
freeznet commented on a change in pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#discussion_r571627498
##########
File path: pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
##########
@@ -417,7 +418,13 @@ public synchronized void restartFunctionInstances(String tenant, String namespac
.type(MediaType.APPLICATION_JSON)
.entity(new ErrorData(fullFunctionName + " has not been assigned yet")).build());
}
- this.functionAdmin.functions().restartFunction(tenant, namespace, functionName);
+ if (ComponentType.SOURCE == componentType) {
Review comment:
following above comment, so we can have the `componentType` as `assignment .getInstance().getFunctionMetaData().getFunctionDetails().getComponentType()`.
##########
File path: pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
##########
@@ -384,7 +385,7 @@ public synchronized void restartFunctionInstance(String tenant, String namespace
}
}
- public synchronized void restartFunctionInstances(String tenant, String namespace, String functionName)
+ public synchronized void restartFunctionInstances(String tenant, String namespace, String functionName, ComponentType componentType)
Review comment:
how about we get `ComponentType` from `Assignment.instance.functionMetaData.functionDetails.ComponentType` instead of passing it as an argument?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] freeznet commented on pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
freeznet commented on pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#issuecomment-774685510
@xche thanks for your help on this issue, i have left some comments, PTAL, thanks.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] nlu90 commented on a change in pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
nlu90 commented on a change in pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#discussion_r572319901
##########
File path: pulsar-functions/worker/src/main/java/org/apache/pulsar/functions/worker/FunctionRuntimeManager.java
##########
@@ -440,8 +450,19 @@ public synchronized void restartFunctionInstances(String tenant, String namespac
}
continue;
}
- this.functionAdmin.functions().restartFunction(tenant, namespace, functionName,
- assignment.getInstance().getInstanceId());
+
+ ComponentType componentType = assignment.getInstance().getFunctionMetaData().getFunctionDetails().getComponentType();
Review comment:
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.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#issuecomment-774679132
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaoran10 commented on pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
gaoran10 commented on pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#issuecomment-776060837
/pulsarbot run-failure-checks
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] jiazhai commented on pull request #9519: [Functions] Call the corresponding restart according to the componenttype. #9502
Posted by GitBox <gi...@apache.org>.
jiazhai commented on pull request #9519:
URL: https://github.com/apache/pulsar/pull/9519#issuecomment-774647756
@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?
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org