[Issue 3851] [pulsar-broker] Support to start broker use the port initialized in cluster metadata#3853
Conversation
sijie
left a comment
sijie
left a comment
There was a problem hiding this comment.
I have a fundamental concern about this change. the port in the service url might be different from the port used in broker at some cases. for example, I have 3 broker listening on 8080. I configure a load balancer over these 3 broker and the load balancer exposes the port at 80. At this case, broker should listen on 8080 but no 80.
So I am not sure if this change is correct. Also bringing configuration store at starting up can be introduce another failure point when a pulsar deployment is over multiple data centers.
| Long zkSessionTimeoutMillis = config.getZooKeeperSessionTimeoutMillis(); | ||
| String clusterName = config.getClusterName(); | ||
| ZooKeeperClientFactory zkfactory = new ZookeeperClientFactoryImpl(); | ||
| ZooKeeper configStoreZk = zkfactory.create( |
There was a problem hiding this comment.
close zookeeper at the end to release resources
|
|
||
| private static int retrievePort(String url) { | ||
| String[] uris = url.split(":"); | ||
| return Integer.parseInt(uris[uris.length-1]); |
There was a problem hiding this comment.
This can be problematic, since the url might potentially contain multiple hosts. since we introduced multiple hosts support in service url since 2.3.0. I would suggest using ServiceURI for parsing the service url and get the list of hosts in the service url.
There was a problem hiding this comment.
thanks for the help.
|
@sijie Thanks for point out these. Is there any other better way to deal with this issue? Or we just need to update the doc Configuring Brokers to guide users to match the service urls that provided when initializing the cluster's metadata? Any help would be appreciated! |
|
@murong00 I think updating the documentation is a better idea for this case. |
|
@sijie Thanks for the advice, please review this change. |
|
@murong00 great contribution! |
|
run java8 tests |
sijie
added
the
doc
|
run java8 tests |
3 similar comments
|
run java8 tests |
|
run java8 tests |
|
run java8 tests |
2 participants
Motivation
Fixes #3851
Modifications
Override the port of service urls in brokerConfig by the config initialized in cluster metadata.