You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@bookkeeper.apache.org by GitBox <gi...@apache.org> on 2022/03/14 12:22:10 UTC
[GitHub] [bookkeeper] gaozhangmin opened a new pull request #3109: Fix getMetadataServiceUri
gaozhangmin opened a new pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109
### Motivation
when config metadataStoreUri like `zk1:2181,zk2:2181/test`, `getMetadataServiceUri ` will only return `zk1:2181`. it's incorrect.
### Changes
Use `getList ` instead of `getString`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on a change in pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825907041
##########
File path: bookkeeper-server/src/main/java/org/apache/bookkeeper/conf/AbstractConfiguration.java
##########
@@ -268,7 +268,11 @@ public String getMetadataServiceUriUnchecked() throws UncheckedConfigurationExce
* @throws ConfigurationException if the metadata service uri is invalid.
*/
public String getMetadataServiceUri() throws ConfigurationException {
- String serviceUri = getString(METADATA_SERVICE_URI);
+ List servers = getList(METADATA_SERVICE_URI, null);
Review comment:
This is not correct, METADATA_SERVICE_URI is a URI, it is not a list of servers
for instance it should start with zk://
##########
File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java
##########
@@ -56,6 +56,12 @@ public void setup() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+ @Test
+ public void testSetGetServiceUri() throws Exception {
+ this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
this is not a valid URI
it should be something like `zk://zk1:2181,zk2:2181/test` or `zk+null://zk1:2181,zk2:2181/test`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin closed pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin closed pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109#issuecomment-1066731314
> I believe you should use the semi colon to separate zk servers: `zk1:2181;zk2:2181/test`
`zk1:2181;zk2:2181/test` is a incorrect connection string, I tried. @eolivelli
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] eolivelli commented on pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109#issuecomment-1066725156
I believe you should use the semi colon to separate zk servers:
`zk1:2181;zk2:2181/test`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825911160
##########
File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java
##########
@@ -56,6 +56,12 @@ public void setup() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+ @Test
+ public void testSetGetServiceUri() throws Exception {
+ this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
https://github.com/apache/pulsar/blob/d02bd7378acd1f202d8bcf50aa6999749e61b20b/pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java#L246-L248
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [bookkeeper] gaozhangmin commented on a change in pull request #3109: Fix getMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #3109:
URL: https://github.com/apache/bookkeeper/pull/3109#discussion_r825909610
##########
File path: bookkeeper-server/src/test/java/org/apache/bookkeeper/conf/AbstractConfigurationTest.java
##########
@@ -56,6 +56,12 @@ public void setup() {
this.conf.setZkLedgersRootPath("/path/to/ledgers");
}
+ @Test
+ public void testSetGetServiceUri() throws Exception {
+ this.conf.setMetadataServiceUri("zk1:2181,zk2:2181/test");
Review comment:
Pulsar broker set serviceUrl like this: `metadata-store:zk:my-zk-1:2181`
--
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.
To unsubscribe, e-mail: issues-unsubscribe@bookkeeper.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org