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 2020/01/03 11:22:28 UTC
[GitHub] [pulsar] murong00 opened a new pull request #5985: [Issue 5935]
Support multi pulsar clusters to use the same bk cluster
murong00 opened a new pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985
### Motivation
Fixes #5935
Support multi pulsar clusters to use the specified bookkeeper cluster by pointing `BookKeeper Client` to the zookeeper connection string of bookkeeper cluster.
### Modifications
Add two configs to recognize bookkeeper cluster metadata store.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #5985: [Issue
5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r379878481
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java
##########
@@ -106,6 +106,14 @@ ClientConfiguration createBkClientConfiguration(ServiceConfiguration conf) {
bkConf.setNettyMaxFrameSizeBytes(conf.getMaxMessageSize() + Commands.MESSAGE_SIZE_FRAME_PADDING);
bkConf.setDiskWeightBasedPlacementEnabled(conf.isBookkeeperDiskWeightBasedPlacementEnabled());
+ if (StringUtils.isNotBlank(conf.getBookkeeperServiceUri())) {
+ bkConf.setMetadataServiceUri(conf.getBookkeeperServiceUri());
+ } else {
+ PulsarService pulsar = new PulsarService(conf);
Review comment:
I don't think we should try to construct PulsarService. because the constructor also does a lot of unrelated things.
I would suggest adding a util function to generate metadata service url from the configuration.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #5985: [Issue
5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r409863585
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1064,6 +1087,27 @@ public String getSafeBrokerServiceUrl() {
return brokerServiceUrl != null ? brokerServiceUrl : brokerServiceUrlTls;
}
+ /**
+ * Get bookkeeper metadata service uri.
+ *
+ * @param config broker configuration
+ * @return the metadata service uri that bookkeeper is used
+ */
+ public static String bookieMetadataServiceUri(ServiceConfiguration config) {
+ ClientConfiguration bkConf = new ClientConfiguration();
+ // init bookkeeper metadata service uri
+ String metadataServiceUri = null;
+ try {
Review comment:
I don't think we should add this logic here. We should reply on the logic that bookkeeper already provides. You can do this by using the following code snippet.
```
ClientConfiguration bkConf = new ClientConfiguration();
bkConf.setZkServers(config.getZooKeeperServers());
return bkConf.getMetadataServiceUri();
```
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-611320579
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-615032422
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r383649876
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/BookKeeperClientFactoryImpl.java
##########
@@ -106,6 +106,14 @@ ClientConfiguration createBkClientConfiguration(ServiceConfiguration conf) {
bkConf.setNettyMaxFrameSizeBytes(conf.getMaxMessageSize() + Commands.MESSAGE_SIZE_FRAME_PADDING);
bkConf.setDiskWeightBasedPlacementEnabled(conf.isBookkeeperDiskWeightBasedPlacementEnabled());
+ if (StringUtils.isNotBlank(conf.getBookkeeperServiceUri())) {
+ bkConf.setMetadataServiceUri(conf.getBookkeeperServiceUri());
+ } else {
+ PulsarService pulsar = new PulsarService(conf);
Review comment:
done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #5985: [Issue
5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r368249653
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -170,16 +170,11 @@ public static void main(String[] args) throws Exception {
ZooKeeper localZk = initZk(arguments.zookeeper, arguments.zkSessionTimeoutMillis);
ZooKeeper configStoreZk = initZk(arguments.configurationStore, arguments.zkSessionTimeoutMillis);
- // Format BookKeeper ledger storage metadata
+ // Format BookKeeper stream storage metadata
ServerConfiguration bkConf = new ServerConfiguration();
bkConf.setZkServers(arguments.zookeeper);
bkConf.setZkTimeout(arguments.zkSessionTimeoutMillis);
- if (localZk.exists("/ledgers", false) == null // only format if /ledgers doesn't exist
Review comment:
why do we remove this logic?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-614997648
@sijie Thanks for the advise and review, please help to take a look.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on issue #5985: [Issue 5935] Support multi
pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-590530179
@murong00 I thought I reviewed it already. but it turned out that I forgot to click submit button. Here is the review comment. PTAL
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-608276549
@jiazhai Thanks for the review, please help to take a look 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 removed a comment on issue #5985: [Issue 5935]
Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 removed a comment on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-611411656
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-615052608
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r368290830
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/PulsarClusterMetadataSetup.java
##########
@@ -170,16 +170,11 @@ public static void main(String[] args) throws Exception {
ZooKeeper localZk = initZk(arguments.zookeeper, arguments.zkSessionTimeoutMillis);
ZooKeeper configStoreZk = initZk(arguments.configurationStore, arguments.zkSessionTimeoutMillis);
- // Format BookKeeper ledger storage metadata
+ // Format BookKeeper stream storage metadata
ServerConfiguration bkConf = new ServerConfiguration();
bkConf.setZkServers(arguments.zookeeper);
bkConf.setZkTimeout(arguments.zkSessionTimeoutMillis);
- if (localZk.exists("/ledgers", false) == null // only format if /ledgers doesn't exist
Review comment:
just a mistake, modified.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-586885399
@sijie @jiazhai I have addressed sijie's comment, please help to review again, 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
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #5985: [Issue
5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r368249756
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -743,6 +743,18 @@
private String kinitCommand = "/usr/bin/kinit";
/**** --- BookKeeper Client --- ****/
+ @FieldContext(
+ category = CATEGORY_STORAGE_BK,
+ doc = "BookKeeper ledgers storage connection string when using a separated BookKeeper cluster."
+ + " If not set it will use local zookeeper quorum of Pulsar cluster"
+ )
+ private String bookkeeperLedgersStore;
Review comment:
in bookkeeper we deprecated using two separate settings. we are encouraging people to use metadata service uri to connect to a zookeeper cluster. we should try to adopt it in Pulsar. Thus if we are adding the support for a separate bookkeeper cluster, let's try to use metadata service uri.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] jiazhai commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r402394408
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/conf/InternalConfigurationData.java
##########
@@ -28,19 +28,19 @@
private String zookeeperServers;
private String configurationStoreServers;
- private String ledgersRootPath;
+ private String metadataServiceUri;
Review comment:
It seems `ledgersRootPath` is much more direct than `metadataServiceUri`. metadata may lead developer think of "zookeeper". How about change this back？
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-613194948
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 removed a comment on issue #5985: [Issue 5935]
Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 removed a comment on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-615013697
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-576000798
@sijie please help to 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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r368290845
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -743,6 +743,18 @@
private String kinitCommand = "/usr/bin/kinit";
/**** --- BookKeeper Client --- ****/
+ @FieldContext(
+ category = CATEGORY_STORAGE_BK,
+ doc = "BookKeeper ledgers storage connection string when using a separated BookKeeper cluster."
+ + " If not set it will use local zookeeper quorum of Pulsar cluster"
+ )
+ private String bookkeeperLedgersStore;
Review comment:
done, 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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-611411656
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r402710498
##########
File path: conf/broker.conf
##########
@@ -461,6 +461,10 @@ httpMaxRequestSize=-1
### --- BookKeeper Client --- ###
+# Metadata service uri that bookkeeper is used for loading corresponding metadata driver
+# and resolving its metadata service location.
+bookkeeperServiceUri=
Review comment:
I will add an example here to make it more clear.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r402709553
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/conf/InternalConfigurationData.java
##########
@@ -28,19 +28,19 @@
private String zookeeperServers;
private String configurationStoreServers;
- private String ledgersRootPath;
+ private String metadataServiceUri;
Review comment:
@jiazhai In fact, `metadataServiceUri` here is a uri to connect to a zookeeper cluster, which is used to find bookies. I would prefer to keep it since `ledgersRootPath` seems more like a path and is using together with zookeeper. As sijie's comment above, we are trying to use a metadata service uri, so how about `bkMetadataServiceUri`?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] jiazhai commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
jiazhai commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-607914893
@murong00 Thanks for the work. left some comments related to the var names. overall lgtm. Would you please take a look, and rebase this PR with the latest master code?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 edited a comment on issue #5985: [Issue 5935]
Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 edited a comment on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-590674620
@sijie Thanks for the review, PTAL.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-590674620
@sijie Thanks for the review, PLAL.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-613219368
@sijie Please help to take a look about this 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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
With regards,
Apache Git Services
[GitHub] [pulsar] sijie commented on a change in pull request #5985: [Issue
5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r409864585
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/conf/InternalConfigurationData.java
##########
@@ -28,19 +28,19 @@
private String zookeeperServers;
private String configurationStoreServers;
- private String ledgersRootPath;
Review comment:
for backward compatibility, you shouldn't change the field directly.
You can add a new field `bookkeeperMetadataServiceUri` and deprecate the old field `ledgersRootPath `.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r402709842
##########
File path: conf/broker.conf
##########
@@ -461,6 +461,10 @@ httpMaxRequestSize=-1
### --- BookKeeper Client --- ###
+# Metadata service uri that bookkeeper is used for loading corresponding metadata driver
+# and resolving its metadata service location.
+bookkeeperServiceUri=
Review comment:
`bkMetadataServiceUri` seems better?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r409950544
##########
File path: pulsar-common/src/main/java/org/apache/pulsar/common/conf/InternalConfigurationData.java
##########
@@ -28,19 +28,19 @@
private String zookeeperServers;
private String configurationStoreServers;
- private String ledgersRootPath;
Review comment:
Thanks for pointing this out, done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r409950462
##########
File path: pulsar-broker/src/main/java/org/apache/pulsar/broker/PulsarService.java
##########
@@ -1064,6 +1087,27 @@ public String getSafeBrokerServiceUrl() {
return brokerServiceUrl != null ? brokerServiceUrl : brokerServiceUrlTls;
}
+ /**
+ * Get bookkeeper metadata service uri.
+ *
+ * @param config broker configuration
+ * @return the metadata service uri that bookkeeper is used
+ */
+ public static String bookieMetadataServiceUri(ServiceConfiguration config) {
+ ClientConfiguration bkConf = new ClientConfiguration();
+ // init bookkeeper metadata service uri
+ String metadataServiceUri = null;
+ try {
Review comment:
Done.
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-615013697
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 removed a comment on issue #5985: [Issue 5935]
Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 removed a comment on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-615032422
/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
With regards,
Apache Git Services
[GitHub] [pulsar] murong00 commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
murong00 commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-589940233
@sijie @jiazhai @tuteng How this issue going, can we finish it in milestone 2.5.1 or 2.6.0?
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] jiazhai commented on a change in pull request #5985:
[Issue 5935] Support multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
jiazhai commented on a change in pull request #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#discussion_r402401397
##########
File path: conf/broker.conf
##########
@@ -461,6 +461,10 @@ httpMaxRequestSize=-1
### --- BookKeeper Client --- ###
+# Metadata service uri that bookkeeper is used for loading corresponding metadata driver
+# and resolving its metadata service location.
+bookkeeperServiceUri=
Review comment:
how about `bkLedgerRootPath`? This seems be more directly.
`bookkeeperServiceUri` may be easy confused with the bookie service url(e.g. "localhost:3181").
----------------------------------------------------------------
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
With regards,
Apache Git Services
[GitHub] [pulsar] tuteng commented on issue #5985: [Issue 5935] Support
multi pulsar clusters to use the same bk cluster
Posted by GitBox <gi...@apache.org>.
tuteng commented on issue #5985: [Issue 5935] Support multi pulsar clusters to use the same bk cluster
URL: https://github.com/apache/pulsar/pull/5985#issuecomment-572064847
run integration tests
run java8 tests
----------------------------------------------------------------
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
With regards,
Apache Git Services