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 2022/03/15 03:12:11 UTC
[GitHub] [pulsar] gaozhangmin opened a new pull request #14686: Fix error setMetadataServiceUri
gaozhangmin opened a new pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686
### Motivation
When use `initialize-cluster-metadata` to init cluster with specify multiple ZK servers. Wrong ZK connection string got when `getMetadataServiceUri` called.
### Modifications
Replace comma with semicolon in ZK connection string when set `setMetadataServiceUri`.
Then replace semicolon with comma when metadata store created.
### Documentation
Check the box below or label this PR directly (if you have committer privilege).
Need to update docs?
- [ ] `doc-required`
(If you need help on updating docs, create a doc issue)
- [ ] `no-need-doc`
(Please explain why)
- [ ] `doc`
(If this PR contains doc changes)
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r829768199
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
@gaozhangmin Have checked this, either `setMetadataServiceUri()` or read from a file both lose the content after the first comma.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828724918
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/AbstractMetadataDriver.java
##########
@@ -98,7 +98,9 @@ void createMetadataStore() throws MetadataException {
String url;
try {
- url = conf.getMetadataServiceUri().replaceFirst(METADATA_STORE_SCHEME + ":", "");
+ url = conf.getMetadataServiceUri()
+ .replaceFirst(METADATA_STORE_SCHEME + ":", "")
+ .replace(";", ",");
Review comment:
If you set `metadataServiceUri` in bookkeeper.conf with semicolon, like zk1:2181;zk2:2181/test, without replace semicolon with comma. metadataStore will be failed created.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r829661507
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
if we are not setting the same option in the Bookie, `metadataServiceUri` in bookkeeper.conf don't support ZK connection string with comma. Only support string with semicolon @eolivelli
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
> I think [apache/bookkeeper#2900](https://github.com/apache/bookkeeper/issues/2900) is not the same as the problem here. [apache/bookkeeper#2900](https://github.com/apache/bookkeeper/issues/2900) caused by the `ServerConfiguration`(extend `CompositeConfiguration`) can't process a comma-separated parameter read from `bk_server.conf`(if you set `zk1:2181,zk2:2181/test` in bk_server.conf, you only get the `zk1:2181` in `ServerConfiguration`).
>
> But here just `ServerConfiguration.setMetadataServiceUri()`, why we will get `["zk1:2181", "zk2:2181/test"].` ?
@aloyszhang You can try `bin/pulsar initialize-cluster-metadata --cluster test --metadata-store zk1:2181,zk2:2181/test --configuration-metadata-store zk1:2181,zk2:2181/test --web-service-url http://localhost:8080/ --broker-service-url pulsar://localhost:6650`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072027555
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r827807032
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
Review comment:
Yes, So I remain the identifier.
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
Review comment:
Yes, So I remain the identifier. @codelipenghui
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jason918 commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
Jason918 commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r827993059
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setMetadataServiceUri("metadata-store:"
Review comment:
The fix seems to be simpler if we just set `bkConf.setDelimiterParsingDisabled(false);`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828059119
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setMetadataServiceUri("metadata-store:"
Review comment:
I am not sure if it has other side effects. @Jason918. it's better use replace, I think.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828717134
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setMetadataServiceUri("metadata-store:"
Review comment:
It should bkConf.setDelimiterParsingDisabled(true), not false. @codelipenghui @Jason918 .
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828971517
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
If a string with comma delimiter. eg zk1:2181,zk2:2181/test. `setMetadataServiceUri ` will get result ["zk1:2181", "zk2:2181/test"]. getMetadataServiceUri only return the first part in list zk1:2181. @eolivelli
Bookeeper also has this problem, see https://github.com/apache/bookkeeper/issues/2900.
Without setDelimiterParsingDisabled, zk connection string with comma is not supported.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828991006
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
I think apache/bookkeeper#2900 is not the same as the problem here. apache/bookkeeper#2900 caused by the
`ServerConfiguration`(extend `CompositeConfiguration`) can't process a comma-separated parameter read from `bk_server.conf`(if you set `zk1:2181,zk2:2181/test` in bk_server.conf, you only get the `zk1:2181` in `ServerConfiguration`).
But here just `ServerConfiguration.setMetadataServiceUri()`, why we will get `["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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang edited a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang edited a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1070806113
Mabye these is another problem if set `MetadataServiceUri` with a prefix `metadata-store:`
```java
bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
```
then execute `BookKeeperAdmin.format`
```java
if (!localStore.exists(BookKeeperConstants.DEFAULT_ZK_LEDGERS_ROOT_PATH).get()
&& !BookKeeperAdmin.format(bkConf, false /* interactive */, false /* force */)) {
throw new IOException("Failed to initialize BookKeeper metadata");
}
```
which will get `MetadataBookieDriver` first
```java
try (MetadataBookieDriver driver = MetadataDrivers.getBookieDriver(
URI.create(conf.getMetadataServiceUri())
))
```
So the URI created here has a scheme `metadata-store`, then getting `MetadataBookieDriver` will throw an exception since bookie only has `zk` and `etcd` as scheme.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1067529509
@gaozhangmin:Thanks for providing doc info!
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828890539
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
why setDelimiterParsingDisabled ?
are we setting it anywhere else ?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828991006
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
I think apache/bookkeeper#2900 is not the same as the problem here. It's because the
`ServerConfiguration`(extend `CompositeConfiguration`) can't process a comma-separated parameter read from `bk_server.conf`(if you set `zk1:2181,zk2:2181/test` in bk_server.conf, you only get the `zk1:2181` in `ServerConfiguration`).
But here just `ServerConfiguration.setMetadataServiceUri()`, why we will get `["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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r829661507
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
if we are not setting the same option in the Bookie, `metadataServiceUri` in bookkeeper.conf don't support ZK connection string with comma. Only support string with semicolon @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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828724918
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/AbstractMetadataDriver.java
##########
@@ -98,7 +98,9 @@ void createMetadataStore() throws MetadataException {
String url;
try {
- url = conf.getMetadataServiceUri().replaceFirst(METADATA_STORE_SCHEME + ":", "");
+ url = conf.getMetadataServiceUri()
+ .replaceFirst(METADATA_STORE_SCHEME + ":", "")
+ .replace(";", ",");
Review comment:
If you set `metadataServiceUri` in bookkeeper.conf with semicolon, like zk1:2181;zk2:2181/test, without replace semicolon with comma. metadataStore will be failed created.
Set `metadataServiceUri` in bookkeeper.conf with comma, it will also cause this issue, Because of BK side
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072387291
@gaozhangmin your answer is not clear to me....
why aren't we adding `setDelimiterParsingDisabled=true` everywhere ?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r829768199
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
@gaozhangmin Have checked this, either `setMetadataServiceUri()` or read from a file both lose the content after the first comma.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1071976491
> Mabye these is another problem if set `MetadataServiceUri` with a prefix `metadata-store:`
>
> ```java
> bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
> ```
>
> then execute `BookKeeperAdmin.format`
>
> ```java
> if (!localStore.exists(BookKeeperConstants.DEFAULT_ZK_LEDGERS_ROOT_PATH).get()
> && !BookKeeperAdmin.format(bkConf, false /* interactive */, false /* force */)) {
> throw new IOException("Failed to initialize BookKeeper metadata");
> }
> ```
>
> which will get `MetadataBookieDriver` first
>
> ```java
> try (MetadataBookieDriver driver = MetadataDrivers.getBookieDriver(
> URI.create(conf.getMetadataServiceUri())
> ))
> ```
>
> So the URI created here has a scheme `metadata-store`, then getting `MetadataBookieDriver` will throw an exception since bookie only has `zk` and `etcd` as scheme.
No, `metadata-store` had been registered. @aloyszhang
```
public class PulsarMetadataBookieDriver extends AbstractMetadataDriver implements MetadataBookieDriver {
// register myself
static {
MetadataDrivers.registerBookieDriver(METADATA_STORE_SCHEME, PulsarMetadataBookieDriver.class);
}
@Override
public MetadataBookieDriver initialize(ServerConfiguration serverConfiguration,
RegistrationManager.RegistrationListener registrationListener,
StatsLogger statsLogger) throws MetadataException {
super.initialize(serverConfiguration);
return this;
}
@Override
public RegistrationManager getRegistrationManager() {
return registrationManager;
}
@Override
public LedgerManagerFactory getLedgerManagerFactory() throws MetadataException {
return ledgerManagerFactory;
}
@Override
public LayoutManager getLayoutManager() {
return layoutManager;
}
}
```
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072000890
Unit test added. @codelipenghui @315157973 @eolivelli @aloyszhang
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072023006
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1070806113
Mabye these is another problem if set `MetadataServiceUri` with a prefix `metadata-store:`
```java
bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
```
then execute `BookKeeperAdmin.format`
```java
if (!localStore.exists(BookKeeperConstants.DEFAULT_ZK_LEDGERS_ROOT_PATH).get()
&& !BookKeeperAdmin.format(bkConf, false /* interactive */, false /* force */)) {
throw new IOException("Failed to initialize BookKeeper metadata");
}
```
which will get `MetadataBookieDriver` first
```java
try (MetadataBookieDriver driver = MetadataDrivers.getBookieDriver(
URI.create(conf.getMetadataServiceUri())
))
```
So the URI created here has a scheme `metadata-store`, getting `MetadataBookieDriver` will throw an exception since bookie only has `zk` and `etcd` as scheme.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli merged pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072021609
Ohh, I see, pulsar register the `metadata-store` scheme from broker side
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin edited a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin edited a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072179865
@eolivelli If we want to support set comma delimiter `metadataServiceUri` configuration in bookkeeper.conf, We should also setDelimiterParsingDisabled =true in this places.
Also, Bookkeeper also should make change on this, as issue https://github.com/apache/bookkeeper/issues/2900 said.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072021609
Ohh, I see, pulsar register the `metadata-store` scheme from broker side
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828993487
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
if we add this option here (bkConf.setDelimiterParsingDisabled(true)), we should set it everywhere, anytime we create a ServerConfiguration object, and this is very hard to maintain.
Are you sure that you are able to run a full Pulsar cluster= if we are not setting the same option in the Bookie it won't start, the same for the Auto Recovery process probably
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1067604528
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r829663601
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
> I think [apache/bookkeeper#2900](https://github.com/apache/bookkeeper/issues/2900) is not the same as the problem here. [apache/bookkeeper#2900](https://github.com/apache/bookkeeper/issues/2900) caused by the `ServerConfiguration`(extend `CompositeConfiguration`) can't process a comma-separated parameter read from `bk_server.conf`(if you set `zk1:2181,zk2:2181/test` in bk_server.conf, you only get the `zk1:2181` in `ServerConfiguration`).
>
> But here just `ServerConfiguration.setMetadataServiceUri()`, why we will get `["zk1:2181", "zk2:2181/test"].` ?
@aloyszhang You can try `bin/pulsar initialize-cluster-metadata --cluster test --metadata-store zk1:2181,zk2:2181/test --configuration-metadata-store zk1:2181,zk2:2181/test --web-service-url http://localhost:8080/ --broker-service-url pulsar://localhost:6650`
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828761819
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/AbstractMetadataDriver.java
##########
@@ -98,7 +98,9 @@ void createMetadataStore() throws MetadataException {
String url;
try {
- url = conf.getMetadataServiceUri().replaceFirst(METADATA_STORE_SCHEME + ":", "");
+ url = conf.getMetadataServiceUri()
+ .replaceFirst(METADATA_STORE_SCHEME + ":", "")
+ .replace(";", ",");
Review comment:
@codelipenghui
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828971517
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
BK setMetadataServiceUri will set a ArrayList, If a string with comma delimiter. eg zk1:2181,zk2:2181/test will get result ["zk1:2181", "zk2:2181/test"]. getMetadataServiceUri only return the first part in list zk1:2181. @eolivelli
Bookeeper also has this problem, see https://github.com/apache/bookkeeper/issues/2900.
Without setDelimiterParsingDisabled, zk connection string with comma is not supported.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang edited a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang edited a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1070806113
Mabye these is another problem if set `MetadataServiceUri` with a prefix `metadata-store:`
```java
bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
```
then execute `BookKeeperAdmin.format`
```java
if (!localStore.exists(BookKeeperConstants.DEFAULT_ZK_LEDGERS_ROOT_PATH).get()
&& !BookKeeperAdmin.format(bkConf, false /* interactive */, false /* force */)) {
throw new IOException("Failed to initialize BookKeeper metadata");
}
```
which will get `MetadataBookieDriver` first
```java
try (MetadataBookieDriver driver = MetadataDrivers.getBookieDriver(
URI.create(conf.getMetadataServiceUri())
))
```
So the URI created here has a scheme `metadata-store`, then getting `MetadataBookieDriver` will throw an exception since bookie only has `zk` and `etcd` as scheme.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828871275
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/AbstractMetadataDriver.java
##########
@@ -98,7 +98,9 @@ void createMetadataStore() throws MetadataException {
String url;
try {
- url = conf.getMetadataServiceUri().replaceFirst(METADATA_STORE_SCHEME + ":", "");
+ url = conf.getMetadataServiceUri()
+ .replaceFirst(METADATA_STORE_SCHEME + ":", "")
+ .replace(";", ",");
Review comment:
> Set metadataServiceUri in bookkeeper.conf with comma, it will also cause this issue, Because of BK side
This is a known issue https://github.com/apache/bookkeeper/issues/2900
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r827792144
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
Review comment:
Nice catch!
I think it should be another BUG here? If we removed the identifier, the `MetadataStoreExtended` will always use zookeeper?
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072457817
> > This pr is aimed to solve failed initialized cluster metadata.
>
> This makes sense to me. but if we merge this and we don't do the other changes in any case users won't be able to run Pulsar with this configuration. So this patch by itself is useless.
Without this change,
1、neither `initialize-cluster-metadata --metadata-store zk:zk1:2181,zk2:2181/test` nor `initialize-cluster-metadata -metadata-store zk:zk1:2181;zk2:2181/test` supported.
2、 neither set `metadataServiceUri="metadata-store:zk:zk1:2181,zk2:2181/test"` nor `metadataServiceUri="metadata-store:zk:zk1:2181;zk2:2181/test"` in bookkeeper.conf supported.
That's what problem this pr tries to solve.
other changes try to support setting `metadataServiceUri="metadata-store:zk:zk1:2181,zk2:2181/test"` in bookkeeper.conf. @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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin edited a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin edited a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072179865
@eolivelli If we want to support set comma delimiter `metadataServiceUri` configuration in bookkeeper.conf, We should also setDelimiterParsingDisabled =true in this places.
Also, Bookkeeper also should make change on this, as issue https://github.com/apache/bookkeeper/issues/2900 said.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin removed a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin removed a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072023006
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072397762
> @gaozhangmin your answer is not clear to me.... why aren't we adding `setDelimiterParsingDisabled=true` everywhere ?
@eolivelli I agree to set setDelimiterParsingDisabled to support comma delimiter. I suggest open a new pr. This pr is aimed to solve failed initialized cluster metadata.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] github-actions[bot] commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1067522374
@gaozhangmin:Thanks for your contribution. For this PR, do we need to update docs?
(The [PR template contains info about doc](https://github.com/apache/pulsar/blob/master/.github/PULL_REQUEST_TEMPLATE.md#documentation), which helps others know more about the changes. Can you provide doc-related info in this and future PR descriptions? 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828105384
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setMetadataServiceUri("metadata-store:"
Review comment:
Looks like it should work, with DelimiterParsing = false, the metadataServiceUri will not be converted to list.
Is it able to add a test? Just to make sure it will not be broken again.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] aloyszhang commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
aloyszhang commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828991006
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -244,7 +244,8 @@ public static void main(String[] args) throws Exception {
// Format BookKeeper ledger storage metadata
if (arguments.existingBkMetadataServiceUri == null && arguments.bookieMetadataServiceUri == null) {
ServerConfiguration bkConf = new ServerConfiguration();
- bkConf.setMetadataServiceUri("metadata-store:" + metadataStoreUrlNoIdentifer);
+ bkConf.setDelimiterParsingDisabled(true);
Review comment:
I think apache/bookkeeper#2900 is not the same as the problem here. apache/bookkeeper#2900 caused by the
`ServerConfiguration`(extend `CompositeConfiguration`) can't process a comma-separated parameter read from `bk_server.conf`
(if you set `zk1:2181,zk2:2181/test` in bk_server.conf, you only get the `zk1:2181` in `ServerConfiguration`).
But here just `ServerConfiguration.setMetadataServiceUri()`, why we will get `["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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1070807465
> I think we should add some unit tests
+1
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on a change in pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on a change in pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#discussion_r828723002
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/bookkeeper/AbstractMetadataDriver.java
##########
@@ -98,7 +98,9 @@ void createMetadataStore() throws MetadataException {
String url;
try {
- url = conf.getMetadataServiceUri().replaceFirst(METADATA_STORE_SCHEME + ":", "");
+ url = conf.getMetadataServiceUri()
+ .replaceFirst(METADATA_STORE_SCHEME + ":", "")
+ .replace(";", ",");
Review comment:
Do we still need to replace it here? Since we removed the replacement before.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin removed a comment on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin removed a comment on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072023006
/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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072179865
@eolivelli If we want to support set comm delimiter `metadataServiceUri` configuration in bookkeeper.conf, We should also setDelimiterParsingDisabled =true in this places.
Also, Bookkeeper also should make change on this as issue https://github.com/apache/bookkeeper/issues/2900 said.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072424481
> This pr is aimed to solve failed initialized cluster metadata.
This makes sense to me.
but if we merge this and we don't do the other changes in any case users won't be able to run Pulsar with this configuration.
So this patch by itself is useless.
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072512551
committed to master branch, thank you @gaozhangmin
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1072387291
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1070807465
> I think we should add some unit tests
+1
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] gaozhangmin commented on pull request #14686: Fix error setMetadataServiceUri
Posted by GitBox <gi...@apache.org>.
gaozhangmin commented on pull request #14686:
URL: https://github.com/apache/pulsar/pull/14686#issuecomment-1071976491
--
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: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org