[functions] Distributed log metadata not correctly initialized by nicoloboschi · Pull Request #13891 · apache/pulsar

nicoloboschi · 2022-01-21T15:31:05Z

Motivation

The PR #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 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
BK client in broker now always use the correct ledgersPath

Documentation

no-need-doc

nicoloboschi · 2022-01-26T05:57:58Z

/pulsarbot run-failure-checks

eolivelli · 2022-01-26T17:00:10Z

@MMirelli is this patch fixing your issue ?

MMirelli · 2022-01-26T17:44:43Z

@MMirelli is this patch fixing your issue ?

Hopefully, I haven't completed the testing yet. But yes it is an attempt to fix #13967.

nicoloboschi · 2022-01-26T20:03:08Z

/pulsarbot run-failure-checks

nicoloboschi · 2022-01-28T19:03:43Z

/pulsarbot rerun-failure-checks

nicoloboschi · 2022-01-28T19:36:19Z

@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

eolivelli

LGTM

lhotari

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 · 2022-02-02T08:21:29Z

@Jason918 would you be able to review these changes? thank you

merlimat · 2022-02-02T17:44:40Z

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.

eolivelli · 2022-02-03T07:44:07Z

I have restarted CI. Merging as soon as it passes

nicoloboschi · 2022-02-03T12:32:39Z

/pulsarbot rerun-failure-checks

lhotari

Good work @nicoloboschi !

…e#13891)

[functions] Distributed log metadata not correctly initialized

5c3b92a

github-actions Bot assigned nicoloboschi Jan 21, 2022

github-actions Bot added the doc-not-needed Your PR changes do not impact docs label Jan 21, 2022

nicoloboschi marked this pull request as draft January 21, 2022 16:59

fix misuse of ZkServers

1c9469a

mattisonchao reviewed Jan 21, 2022

View reviewed changes

Comment thread pulsar-broker/src/test/java/org/apache/pulsar/broker/zookeeper/ClusterMetadataSetupTest.java Outdated

nicoloboschi added 4 commits January 26, 2022 00:27

debug and fix

9aea13b

remove jaars

043c453

fix broker

aa75ceb

checkstyle

17cee4e

fix bk format

cbf0fc3

nicoloboschi mentioned this pull request Jan 26, 2022

[Functions] ZK metadata badly initialised after PR13296 #13967

Closed

real fix - use metadataStoreUrl syntax

1b97fe0

eolivelli reviewed Jan 26, 2022

View reviewed changes

Comment thread pulsar-broker/src/test/java/org/apache/pulsar/PulsarClusterMetadataSetupTest.java Outdated

eolivelli reviewed Jan 26, 2022

View reviewed changes

Comment thread pulsar-metadata/src/main/java/org/apache/pulsar/metadata/impl/ZKMetadataStore.java Outdated

eolivelli requested changes Jan 26, 2022

View reviewed changes

Comment thread pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java Outdated

nicoloboschi added 3 commits January 26, 2022 16:04

refactor and fix for dlog

db6b28e

fix checkstyle

dd8ca70

checkstyle

52bf3f4

fix wrong unit tests

e780274

nicoloboschi marked this pull request as ready for review January 26, 2022 20:03

nicoloboschi added 3 commits January 27, 2022 15:51

fix chroot

2b24c7c

checkstyle

7411ec9

Merge branch 'master' into fix/dlog-initializiation

55d4bcc

eolivelli requested a review from merlimat January 27, 2022 20:08

fix bk ledgers format

a2170a4

checkstyle

1de9a1f

eolivelli approved these changes Jan 29, 2022

View reviewed changes

eolivelli requested a review from codelipenghui January 29, 2022 13:40

codelipenghui added this to the 2.10.0 milestone Jan 30, 2022

lhotari reviewed Jan 31, 2022

View reviewed changes

lhotari requested a review from Jason918 February 2, 2022 08:21

lhotari requested review from addisonj, hezhangjian and sijie February 2, 2022 08:25

hezhangjian approved these changes Feb 2, 2022

View reviewed changes

merlimat approved these changes Feb 2, 2022

View reviewed changes

Comment thread pulsar-broker-common/src/main/java/org/apache/pulsar/broker/ServiceConfiguration.java

nicoloboschi added 2 commits February 2, 2022 22:25

add test for metadata-store and fix metadata-store usage

b72da20

fix tests and cleanup

e5ad2be

Jason918 approved these changes Feb 3, 2022

View reviewed changes

lhotari approved these changes Feb 3, 2022

View reviewed changes

eolivelli merged commit 25db606 into apache:master Feb 3, 2022

Nicklee007 pushed a commit to Nicklee007/pulsar that referenced this pull request Apr 20, 2022

[functions] Distributed log metadata not correctly initialized (apach…

006cbb1

…e#13891)

Uh oh!

Conversation

nicoloboschi commented Jan 21, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Motivation

Modifications

Documentation

Uh oh!

Uh oh!

nicoloboschi commented Jan 26, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

eolivelli commented Jan 26, 2022

Uh oh!

MMirelli commented Jan 26, 2022

Uh oh!

nicoloboschi commented Jan 26, 2022

Uh oh!

nicoloboschi commented Jan 28, 2022

Uh oh!

nicoloboschi commented Jan 28, 2022

Uh oh!

eolivelli left a comment

Choose a reason for hiding this comment

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

lhotari commented Feb 2, 2022

Uh oh!

merlimat commented Feb 2, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

eolivelli commented Feb 3, 2022

Uh oh!

nicoloboschi commented Feb 3, 2022

Uh oh!

lhotari left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

nicoloboschi commented Jan 21, 2022 •

edited

Loading

merlimat commented Feb 2, 2022 •

edited

Loading