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/12/21 10:51:39 UTC
[GitHub] [pulsar] eolivelli opened a new pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses
eolivelli opened a new pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019
This change allows ZkBookieRackAffinityMapping to deal with BookieId instead of raw BookieSocketAddresses.
Summary of changes:
- upgrade to BK 4.12.1 (still SNAPSHOT, so this patch is to be considered a preview)
- use BookieAddressResolver in ZkBookieRackAffinityMapping
- Start BookieServer passing "null" as BookieEndpointInfo instead of BookieEndpointInfo.NO_INFO (this change allows the Bookie bundled with Pulsar to publish local endpoints)
### Documentation
- Does this pull request introduce a new feature? yes
- If yes, how is the feature documented? not documented
- If a feature is not documented yet in this PR, please create a followup issue for adding the documentation
I will create a followup issue once the patch is ready to be reviewed and merged
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] sijie commented on a change in pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses
Posted by GitBox <gi...@apache.org>.
sijie commented on a change in pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#discussion_r554705648
##########
File path: conf/bookkeeper.conf
##########
@@ -23,6 +23,10 @@
## Server parameters
#############################################################################
+# Bookie identifier, by default it is computed using the advertised bookies address
+# This value cannot be changed once the bookie is started for the first time
+#bookieId=
Review comment:
We need clear documentation on how to use this and what is the path for migrating the existing cluster to use BookieId.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
codelipenghui commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-760019561
@eolivelli Merged
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-755984688
This patch is waiting for BK 4.12.1 release, a VOTE is pending
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-758763905
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] Jennifer88huang commented on a change in pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
Jennifer88huang commented on a change in pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#discussion_r555561260
##########
File path: distribution/server/src/assemble/LICENSE.bin.txt
##########
@@ -476,9 +476,9 @@ The Apache Software License, Version 2.0
- org.apache.avro-avro-1.9.1.jar
- org.apache.avro-avro-protobuf-1.9.1.jar
* Apache Curator
- - org.apache.curator-curator-client-4.0.1.jar
- - org.apache.curator-curator-framework-4.0.1.jar
- - org.apache.curator-curator-recipes-4.0.1.jar
+ - org.apache.curator-curator-client-5.1.0.jar
+ - org.apache.curator-curator-framework-5.1.0.jar
+ - org.apache.curator-curator-recipes-5.1.0.jar
Review comment:
same concern here.
##########
File path: distribution/server/src/assemble/LICENSE.bin.txt
##########
@@ -395,32 +395,32 @@ The Apache Software License, Version 2.0
- org.apache.logging.log4j-log4j-1.2-api-2.14.0.jar
* Java Native Access JNA -- net.java.dev.jna-jna-4.2.0.jar
* BookKeeper
- - org.apache.bookkeeper-bookkeeper-common-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-common-allocator-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-proto-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-server-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-tools-framework-4.12.0.jar
- - org.apache.bookkeeper-circe-checksum-4.12.0.jar
- - org.apache.bookkeeper-cpu-affinity-4.12.0.jar
- - org.apache.bookkeeper-statelib-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-api-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-common-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-java-client-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-java-client-base-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-proto-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-server-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-service-api-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar
- - org.apache.bookkeeper.http-http-server-4.12.0.jar
- - org.apache.bookkeeper.http-vertx-http-server-4.12.0.jar
- - org.apache.bookkeeper.stats-bookkeeper-stats-api-4.12.0.jar
- - org.apache.bookkeeper.stats-prometheus-metrics-provider-4.12.0.jar
- - org.apache.bookkeeper.tests-stream-storage-tests-common-4.12.0.jar
- - org.apache.distributedlog-distributedlog-common-4.12.0.jar
- - org.apache.distributedlog-distributedlog-core-4.12.0-tests.jar
- - org.apache.distributedlog-distributedlog-core-4.12.0.jar
- - org.apache.distributedlog-distributedlog-protocol-4.12.0.jar
- - org.apache.bookkeeper.stats-codahale-metrics-provider-4.12.0.jar
+ - org.apache.bookkeeper-bookkeeper-common-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-common-allocator-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-proto-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-server-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-tools-framework-4.12.1.jar
+ - org.apache.bookkeeper-circe-checksum-4.12.1.jar
+ - org.apache.bookkeeper-cpu-affinity-4.12.1.jar
+ - org.apache.bookkeeper-statelib-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-api-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-common-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-java-client-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-java-client-base-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-proto-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-server-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-service-api-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-service-impl-4.12.1.jar
+ - org.apache.bookkeeper.http-http-server-4.12.1.jar
+ - org.apache.bookkeeper.http-vertx-http-server-4.12.1.jar
+ - org.apache.bookkeeper.stats-bookkeeper-stats-api-4.12.1.jar
+ - org.apache.bookkeeper.stats-prometheus-metrics-provider-4.12.1.jar
+ - org.apache.bookkeeper.tests-stream-storage-tests-common-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-common-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-core-4.12.1-tests.jar
+ - org.apache.distributedlog-distributedlog-core-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-protocol-4.12.1.jar
+ - org.apache.bookkeeper.stats-codahale-metrics-provider-4.12.1.jar
Review comment:
@eolivelli thank you for your contribution.
I was wondering do we have a way to keep it as a variable, so it will be changed with the bk release automatically in the future. Otherwise, we might need to manually update the versions.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#discussion_r555044903
##########
File path: conf/bookkeeper.conf
##########
@@ -23,6 +23,10 @@
## Server parameters
#############################################################################
+# Bookie identifier, by default it is computed using the advertised bookies address
+# This value cannot be changed once the bookie is started for the first time
+#bookieId=
Review comment:
sure, let's remove this line for the scope of this patch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on a change in pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#discussion_r555563128
##########
File path: distribution/server/src/assemble/LICENSE.bin.txt
##########
@@ -395,32 +395,32 @@ The Apache Software License, Version 2.0
- org.apache.logging.log4j-log4j-1.2-api-2.14.0.jar
* Java Native Access JNA -- net.java.dev.jna-jna-4.2.0.jar
* BookKeeper
- - org.apache.bookkeeper-bookkeeper-common-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-common-allocator-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-proto-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-server-4.12.0.jar
- - org.apache.bookkeeper-bookkeeper-tools-framework-4.12.0.jar
- - org.apache.bookkeeper-circe-checksum-4.12.0.jar
- - org.apache.bookkeeper-cpu-affinity-4.12.0.jar
- - org.apache.bookkeeper-statelib-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-api-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-common-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-java-client-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-java-client-base-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-proto-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-server-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-service-api-4.12.0.jar
- - org.apache.bookkeeper-stream-storage-service-impl-4.12.0.jar
- - org.apache.bookkeeper.http-http-server-4.12.0.jar
- - org.apache.bookkeeper.http-vertx-http-server-4.12.0.jar
- - org.apache.bookkeeper.stats-bookkeeper-stats-api-4.12.0.jar
- - org.apache.bookkeeper.stats-prometheus-metrics-provider-4.12.0.jar
- - org.apache.bookkeeper.tests-stream-storage-tests-common-4.12.0.jar
- - org.apache.distributedlog-distributedlog-common-4.12.0.jar
- - org.apache.distributedlog-distributedlog-core-4.12.0-tests.jar
- - org.apache.distributedlog-distributedlog-core-4.12.0.jar
- - org.apache.distributedlog-distributedlog-protocol-4.12.0.jar
- - org.apache.bookkeeper.stats-codahale-metrics-provider-4.12.0.jar
+ - org.apache.bookkeeper-bookkeeper-common-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-common-allocator-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-proto-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-server-4.12.1.jar
+ - org.apache.bookkeeper-bookkeeper-tools-framework-4.12.1.jar
+ - org.apache.bookkeeper-circe-checksum-4.12.1.jar
+ - org.apache.bookkeeper-cpu-affinity-4.12.1.jar
+ - org.apache.bookkeeper-statelib-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-api-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-common-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-java-client-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-java-client-base-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-proto-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-server-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-service-api-4.12.1.jar
+ - org.apache.bookkeeper-stream-storage-service-impl-4.12.1.jar
+ - org.apache.bookkeeper.http-http-server-4.12.1.jar
+ - org.apache.bookkeeper.http-vertx-http-server-4.12.1.jar
+ - org.apache.bookkeeper.stats-bookkeeper-stats-api-4.12.1.jar
+ - org.apache.bookkeeper.stats-prometheus-metrics-provider-4.12.1.jar
+ - org.apache.bookkeeper.tests-stream-storage-tests-common-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-common-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-core-4.12.1-tests.jar
+ - org.apache.distributedlog-distributedlog-core-4.12.1.jar
+ - org.apache.distributedlog-distributedlog-protocol-4.12.1.jar
+ - org.apache.bookkeeper.stats-codahale-metrics-provider-4.12.1.jar
Review comment:
this approach could be feasible, but we should do it for all of the dependencies,
currently we have this manual approach
and personally I am in favour of having something to do manually in this case
because you are required to validate manually the updates to third party dependencies.
we could create a new task in order to automate the generation of this file
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-759362257
@sijie @codelipenghui CI passed, can you please merge ?
I will follow up with rebasing the upgrade of ZK, that depends on this patch
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-757956833
@sijie I have removed the configuration entry and created the issue for the docs
https://github.com/apache/pulsar/issues/9174
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-758535122
/pulsarbot run-failure-checks
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] codelipenghui merged pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
codelipenghui merged pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] eolivelli commented on pull request #9019: Allow Pulsar to use BookieID instead of Bookie Network Addresses (Update BK to 4.12.1)
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #9019:
URL: https://github.com/apache/pulsar/pull/9019#issuecomment-760025958
cool!
thank you very much @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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org