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/01/21 15:31:20 UTC
[GitHub] [pulsar] nicoloboschi opened a new pull request #13891: [functions] Distributed log metadata not correctly initialized
nicoloboschi opened a new pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891
### Motivation
The PR https://github.com/apache/pulsar/pull/13296 introduces a bug that breaks the functions worker in certain types of configuration.
To reproduce:
* Use `zookkeeper` entry in `initialize-cluster-metadata` command (which is the common configuration before PIP-45) (see the unit test for reference)
* With [this](https://github.com/apache/pulsar/pull/13296/files#diff-03a1201aab4aa284723e575a7c9b18ad9542fcc02beb80d83cfb87d64adf54b5R234
) change the DL configuration does not use the correct ledgersPath (`/ledgers` by default) anymore and instead, it uses the root path (ledgersPath="").
* Start functions-worker and install a connector. This trigger the DLog client to upload the Nar file to the bookies. Since the ledgers path does not match with the actual BK ledgers path, the client is not able to find an available bookie.
```
java.io.IOException: org.apache.bookkeeper.client.BKException$ZKException: Error while using ZooKeeper
at org.apache.distributedlog.BookKeeperClient.commonInitialization(BookKeeperClient.java:123) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BookKeeperClient.initialize(BookKeeperClient.java:172) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BookKeeperClient.get(BookKeeperClient.java:199) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BookKeeperClient.createLedger(BookKeeperClient.java:211) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.bk.SimpleLedgerAllocator.allocateLedger(SimpleLedgerAllocator.java:370) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.bk.SimpleLedgerAllocator.allocate(SimpleLedgerAllocator.java:271) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.bk.LedgerAllocatorDelegator.allocate(LedgerAllocatorDelegator.java:67) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.impl.logsegment.BKLogSegmentAllocator.allocate(BKLogSegmentAllocator.java:55) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BKLogWriteHandler.doStartLogSegment(BKLogWriteHandler.java:571) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BKLogWriteHandler$10.onSuccess(BKLogWriteHandler.java:538) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BKLogWriteHandler$10.onSuccess(BKLogWriteHandler.java:530) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.bookkeeper.common.concurrent.FutureEventListener.accept(FutureEventListener.java:42) ~[org.apache.bookkeeper-bookkeeper-common-4.14.4.jar:4.14.4]
at org.apache.bookkeeper.common.concurrent.FutureEventListener.accept(FutureEventListener.java:26) ~[org.apache.bookkeeper-bookkeeper-common-4.14.4.jar:4.14.4]
at java.util.concurrent.CompletableFuture.uniWhenComplete(CompletableFuture.java:859) ~[?:?]
at java.util.concurrent.CompletableFuture.uniWhenCompleteStage(CompletableFuture.java:883) ~[?:?]
at java.util.concurrent.CompletableFuture.whenComplete(CompletableFuture.java:2251) ~[?:?]
at org.apache.distributedlog.BKLogWriteHandler.asyncStartLogSegment(BKLogWriteHandler.java:530) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at org.apache.distributedlog.BKAbstractLogWriter.lambda$asyncStartNewLogSegment$1(BKAbstractLogWriter.java:379) ~[org.apache.distributedlog-distributedlog-core-4.14.4.jar:4.14.4]
at java.util.concurrent.CompletableFuture$UniCompose.tryFire(CompletableFuture.java:1072) [?:?]
at java.util.concurrent.CompletableFuture.postComplete(CompletableFuture.java:506) [?:?]
at java.util.concurrent.CompletableFuture.complete(CompletableFuture.java:2073) [?:?]
at org.apache.bookkeeper.common.concurrent.FutureUtils$ListFutureProcessor.run(FutureUtils.java:229) [org.apache.bookkeeper-bookkeeper-common-4.14.4.jar:4.14.4]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
at com.google.common.util.concurrent.TrustedListenableFutureTask$TrustedFutureInterruptibleTask.runInterruptibly(TrustedListenableFutureTask.java:125) [com.google.guava-guava-30.1-jre.jar:?]
at com.google.common.util.concurrent.InterruptibleTask.run(InterruptibleTask.java:69) [com.google.guava-guava-30.1-jre.jar:?]
at com.google.common.util.concurrent.TrustedListenableFutureTask.run(TrustedListenableFutureTask.java:78) [com.google.guava-guava-30.1-jre.jar:?]
at java.util.concurrent.Executors$RunnableAdapter.call(Executors.java:515) [?:?]
at java.util.concurrent.FutureTask.run(FutureTask.java:264) [?:?]
at java.util.concurrent.ScheduledThreadPoolExecutor$ScheduledFutureTask.run(ScheduledThreadPoolExecutor.java:304) [?:?]
at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1128) [?:?]
at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:628) [?:?]
at io.netty.util.concurrent.FastThreadLocalRunnable.run(FastThreadLocalRunnable.java:30) [io.netty-netty-common-4.1.72.Final.jar:4.1.72.Final]
at java.lang.Thread.run(Thread.java:829) [?:?]
Caused by: org.apache.bookkeeper.client.BKException$ZKException: Error while using ZooKeeper
at org.apache.bookkeeper.discover.ZKRegistrationClient.lambda$getChildren$4(ZKRegistrationClient.java:351) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
at org.apache.bookkeeper.zookeeper.ZooKeeperClient$25$1.processResult(ZooKeeperClient.java:1177) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:678) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:563) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
Caused by: org.apache.zookeeper.KeeperException$NoNodeException: KeeperErrorCode = NoNode for /available
at org.apache.zookeeper.KeeperException.create(KeeperException.java:118) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
at org.apache.zookeeper.KeeperException.create(KeeperException.java:54) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
at org.apache.bookkeeper.discover.ZKRegistrationClient.lambda$getChildren$4(ZKRegistrationClient.java:350) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
at org.apache.bookkeeper.zookeeper.ZooKeeperClient$25$1.processResult(ZooKeeperClient.java:1177) ~[org.apache.bookkeeper-bookkeeper-server-4.14.4.jar:4.14.4]
at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:678) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:563) ~[org.apache.zookeeper-zookeeper-3.6.3.jar:3.6.3]
```
### Modifications
* Pass the correct ledgersPath during the cluster metadata initialization
### Documentation
- [x] `no-need-doc`
--
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 #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r792643068
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -2580,7 +2581,14 @@ public String getBookkeeperMetadataStoreUrl() {
} else {
// Fallback to same metadata service used by broker, adding the "metadata-store" to specify the BK
// metadata adapter
- return "metadata-store:" + getMetadataStoreUrl();
+ String suffix;
+ if (StringUtils.isNotBlank(metadataStoreUrl)) {
+ suffix = metadataStoreUrl;
+ } else {
+ // Fallback to old setting
+ suffix = "zk:" + zookeeperServers + new ClientConfiguration().getZkLedgersRootPath();
Review comment:
should we use some utility method in ClientConfiguration ?
or build a dummy ClientConfiguration and use BK functions to compute the value ?
--
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] merlimat commented on a change in pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
merlimat commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r797866198
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -2579,7 +2580,15 @@ public String getBookkeeperMetadataStoreUrl() {
} else {
// Fallback to same metadata service used by broker, adding the "metadata-store" to specify the BK
// metadata adapter
- return "metadata-store:" + getMetadataStoreUrl();
+ String suffix;
+ if (StringUtils.isNotBlank(metadataStoreUrl)) {
Review comment:
wouldn't we have the same issue here if a user configure `metadataStoreUrl=my-zk:2181`? Since we're not requiring it even there.
--
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] nicoloboschi commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1028944239
/pulsarbot rerun-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] eolivelli commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1028690444
I have restarted CI. Merging as soon as it passes
--
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 #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli merged pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891
--
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 #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r792642225
##########
File path: pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/ZKMetadataStore.java
##########
@@ -507,7 +508,7 @@ public ZooKeeper getZkClient() {
int chrootIndex = metadataURL.indexOf("/");
if (chrootIndex > 0) {
String chrootPath = metadataURL.substring(chrootIndex);
- String zkConnectForChrootCreation = metadataURL.substring(0, chrootIndex);
+ String zkConnectForChrootCreation = metadataURL.substring(0, chrootIndex).replace("zk:", "");
Review comment:
we should drop "zk:" only if it is in the beginning of the string
--
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] merlimat commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
merlimat commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1028191762
> It might not be related to this PR, but I don't understand the format of the `metadata-store:` URL. It's really odd that `zk:` in `metadata-store:zk:` seems to be optional. @merlimat What's the reasoning for this? Can you also review this PR?
Main reason was to accept `my-zk:2181` and interpret it as a ZK address for backward compatibility. Eg: if you're passing just upgrading and you have the `zookeeperServers`.
When configuring BK access, it has already its own prefix for the BK pluggable metadata. There we use the prefix `metadata-store:` to load the Pulsar plugin. Everything after that should be the same that the user has configured for Pulsar broker.
--
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] nicoloboschi commented on a change in pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r797954319
##########
File path: pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java
##########
@@ -2579,7 +2580,15 @@ public String getBookkeeperMetadataStoreUrl() {
} else {
// Fallback to same metadata service used by broker, adding the "metadata-store" to specify the BK
// metadata adapter
- return "metadata-store:" + getMetadataStoreUrl();
+ String suffix;
+ if (StringUtils.isNotBlank(metadataStoreUrl)) {
Review comment:
It's true, I'll add a couple of tests. it seems not covered at all now
--
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] mattisonchao commented on a change in pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
mattisonchao commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r790065901
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ClusterMetadataSetupTest.java
##########
@@ -26,25 +26,34 @@
import java.io.File;
import java.io.IOException;
import java.net.InetSocketAddress;
+import java.net.URI;
import java.nio.charset.Charset;
import java.util.Arrays;
import java.util.HashSet;
import java.util.List;
import java.util.SortedMap;
import java.util.TreeMap;
+
Review comment:
```suggestion
```
--
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 #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli commented on a change in pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#discussion_r792641134
##########
File path: pulsar-broker/src/test/java/org/apache/pulsar/PulsarClusterMetadataSetupTest.java
##########
@@ -59,4 +63,12 @@ public void testMainGenerateDocs() throws Exception {
System.setOut(oldStream);
}
}
+
+ @Test
+ public void testToDelete() throws Exception {
+ System.setProperty("bookkeeper.metadata.bookie.drivers", PulsarMetadataBookieDriver.class.getName());
Review comment:
please us System.clearProperty to clean up the env
--
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] nicoloboschi commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1021890931
/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] nicoloboschi commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1022555766
/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] nicoloboschi commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1024552157
@MMirelli I've run the test you mentioned and now it works with these changes. Also the integration tests are passing
@eolivelli @lhotari @merlimat @michaeljmarshall 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.
To unsubscribe, e-mail: commits-unsubscribe@pulsar.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [pulsar] lhotari commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
lhotari commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1027688969
@Jason918 would you be able to review these changes? thank you
--
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] merlimat edited a comment on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
merlimat edited a comment on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1028191762
> It might not be related to this PR, but I don't understand the format of the `metadata-store:` URL. It's really odd that `zk:` in `metadata-store:zk:` seems to be optional. @merlimat What's the reasoning for this? Can you also review this PR?
@lhotari Main reason was to accept `my-zk:2181` and interpret it as a ZK address for backward compatibility. Eg: if you're passing just upgrading and you have the `zookeeperServers`.
When configuring BK access, it has already its own prefix for the BK pluggable metadata. There we use the prefix `metadata-store:` to load the Pulsar plugin. Everything after that should be the same that the user has configured for Pulsar broker.
--
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] nicoloboschi commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
nicoloboschi commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1024520597
/pulsarbot rerun-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] eolivelli commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
eolivelli commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1022398410
@MMirelli is this patch fixing your issue ?
--
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] MMirelli commented on pull request #13891: [functions] Distributed log metadata not correctly initialized
Posted by GitBox <gi...@apache.org>.
MMirelli commented on pull request #13891:
URL: https://github.com/apache/pulsar/pull/13891#issuecomment-1022438176
> @MMirelli is this patch fixing your issue ?
Hopefully, I haven't completed the testing yet. But yes it is an attempt to fix #13967.
--
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