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/05/12 08:21:12 UTC
[GitHub] [pulsar] wangjialing218 opened a new pull request #10552: get function cluster from broker config when start function worker with broker
wangjialing218 opened a new pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552
### Motivation
When start function worker with broker, we need to set pulsarFunctionsCluster in functions_worker.yml, other wise it will fail to start. #2328
If we run broker in k8s, we should set correct config map to change the default value in functions_worker.yml.
In this mode, pulsarFunctionsCluster should always be same with the cluster name in broker.conf, so we can get the setting directly from broker.conf.
### Modifications
get function cluster from broker config when start function worker with broker
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-840208961
> if `setPulsarFunctionsCluster` is required for making the broker work.
> do we have test cases that set that value ?
> we have to fix all of those tests and remove the call to `setPulsarFunctionsCluster`
> does it make sense to you ?
>
> it looks like it is useless to set a value explicitly if you cannot change it
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-840219636
> if `setPulsarFunctionsCluster` is required for making the broker work.
> do we have test cases that set that value ?
> we have to fix all of those tests and remove the call to `setPulsarFunctionsCluster`
> does it make sense to you ?
>
@eolivelli I have modify exist test case to meet this change, please have a check
> it looks like it is useless to set a value explicitly if you cannot change it
>
Currently default value of FunctionsCluster is loaded from `functions_worker.yml`.
When we start function woker separately, we need to set FunctionsCluster in `functions_worker.yml` because we can not get cluster name directly from broker.
And when we start function worker with broker, we can get cluster name directly from broker config, no need to load the value from ‘functions_worker.yml’.
--
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 #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
sijie commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-841637101
@eolivelli please review this.
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-840358137
/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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-842041146
/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] wangjialing218 closed pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 closed pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-841920302
/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] wangjialing218 removed a comment on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 removed a comment on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-840208961
> if `setPulsarFunctionsCluster` is required for making the broker work.
> do we have test cases that set that value ?
> we have to fix all of those tests and remove the call to `setPulsarFunctionsCluster`
> does it make sense to you ?
>
> it looks like it is useless to set a value explicitly if you cannot change it
--
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] eolivelli commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-839695105
if `setPulsarFunctionsCluster` is required for making the broker work.
do we have test cases that set that value ?
we have to fix all of those tests and remove the call to `setPulsarFunctionsCluster`
does it make sense to you ?
it looks like it is useless to set a value explicitly if you cannot change it
--
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] wangjialing218 commented on a change in pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on a change in pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#discussion_r630905878
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1515,6 +1515,7 @@ public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfigu
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
ServiceConfigurationUtils.getAppliedAdvertisedAddress(brokerConfig));
workerConfig.setWorkerHostname(hostname);
+ workerConfig.setPulsarFunctionsCluster(brokerConfig.getClusterName());
Review comment:
If user configure another value different with brokerConfig.getClusterName() , broker will fail to start, refer to #2328.
To avoid this to happen, we could force the value same with brokerConfig.getClusterName().
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-842749716
/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] eolivelli commented on a change in pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#discussion_r630902440
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1515,6 +1515,7 @@ public static WorkerConfig initializeWorkerConfigFromBrokerConfig(ServiceConfigu
String hostname = ServiceConfigurationUtils.getDefaultOrConfiguredAddress(
ServiceConfigurationUtils.getAppliedAdvertisedAddress(brokerConfig));
workerConfig.setWorkerHostname(hostname);
+ workerConfig.setPulsarFunctionsCluster(brokerConfig.getClusterName());
Review comment:
what about setting this `brokerConfig.getClusterName()` value only if the user did not configure another value ?
otherwise this change may introduce a behaviour change
--
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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-840316637
/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] wangjialing218 commented on pull request #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
wangjialing218 commented on pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552#issuecomment-841920302
--
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 #10552: get function cluster from broker config when start function worker with broker
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #10552:
URL: https://github.com/apache/pulsar/pull/10552
--
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