Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[managed-ledger] create bookkeeper client during ManagedLedger shutdown #4573

Closed
yjshen opened this issue Jun 21, 2019 · 3 comments
Closed
Assignees
Labels
type/bug The PR fixed a bug or issue reported a bug
Milestone

Comments

@yjshen
Copy link
Member

yjshen commented Jun 21, 2019

During ManagedLedgerFactoryImpl.shutdown(), the zookeeper client is closed first and when isBookkeeperManager, it will get a bookkeeper from bookkeeperFactory.get() which result in creating new BookKeeper client using closed zk client, which caused this bug.

java.lang.IllegalStateException: java.io.IOException: org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss

	at org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.lambda$new$0(ManagedLedgerFactoryImpl.java:125)
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl$BookkeeperFactoryForCustomEnsemblePlacementPolicy.get(ManagedLedgerFactoryImpl.java:614)
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.shutdown(ManagedLedgerFactoryImpl.java:431)
	at org.apache.pulsar.segment.pulsar.PulsarSegmentSourceBuilder.build(PulsarSegmentSourceBuilder.java:143)
	at org.apache.pulsar.segment.impl.SegmentEntryReaderImplTest.getNonPartitionedTopicSegments(SegmentEntryReaderImplTest.java:137)
	at org.apache.pulsar.segment.impl.SegmentEntryReaderImplTest.testReadEntriesFromCompleteLogSegment(SegmentEntryReaderImplTest.java:151)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:498)
	at org.junit.runners.model.FrameworkMethod$1.runReflectiveCall(FrameworkMethod.java:50)
	at org.junit.internal.runners.model.ReflectiveCallable.run(ReflectiveCallable.java:12)
	at org.junit.runners.model.FrameworkMethod.invokeExplosively(FrameworkMethod.java:47)
	at org.junit.internal.runners.statements.InvokeMethod.evaluate(InvokeMethod.java:17)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.rules.TestWatcher$1.evaluate(TestWatcher.java:55)
	at org.junit.rules.RunRules.evaluate(RunRules.java:20)
	at org.junit.runners.ParentRunner.runLeaf(ParentRunner.java:325)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:78)
	at org.junit.runners.BlockJUnit4ClassRunner.runChild(BlockJUnit4ClassRunner.java:57)
	at org.junit.runners.ParentRunner$3.run(ParentRunner.java:290)
	at org.junit.runners.ParentRunner$1.schedule(ParentRunner.java:71)
	at org.junit.runners.ParentRunner.runChildren(ParentRunner.java:288)
	at org.junit.runners.ParentRunner.access$000(ParentRunner.java:58)
	at org.junit.runners.ParentRunner$2.evaluate(ParentRunner.java:268)
	at org.junit.internal.runners.statements.RunBefores.evaluate(RunBefores.java:26)
	at org.junit.internal.runners.statements.RunAfters.evaluate(RunAfters.java:27)
	at org.junit.runners.ParentRunner.run(ParentRunner.java:363)
	at org.junit.runner.JUnitCore.run(JUnitCore.java:137)
	at com.intellij.junit4.JUnit4IdeaTestRunner.startRunnerWithArgs(JUnit4IdeaTestRunner.java:68)
	at com.intellij.rt.execution.junit.IdeaTestRunner$Repeater.startRunnerWithArgs(IdeaTestRunner.java:47)
	at com.intellij.rt.execution.junit.JUnitStarter.prepareStreamsAndStart(JUnitStarter.java:242)
	at com.intellij.rt.execution.junit.JUnitStarter.main(JUnitStarter.java:70)
Caused by: java.io.IOException: org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss
	at org.apache.bookkeeper.client.BookKeeper.validateZooKeeper(BookKeeper.java:342)
	at org.apache.bookkeeper.client.BookKeeper.<init>(BookKeeper.java:368)
	at org.apache.bookkeeper.mledger.impl.ManagedLedgerFactoryImpl.lambda$new$0(ManagedLedgerFactoryImpl.java:123)
	... 33 more
Caused by: org.apache.zookeeper.KeeperException$ConnectionLossException: KeeperErrorCode = ConnectionLoss
	at org.apache.zookeeper.KeeperException.create(KeeperException.java:102)
	... 36 more
@yjshen yjshen added the type/bug The PR fixed a bug or issue reported a bug label Jun 21, 2019
@yjshen
Copy link
Member Author

yjshen commented Jun 21, 2019

@rdhabalia could you please take a look at this? It seems it's caused by #3933?

    public void shutdown() throws InterruptedException, ManagedLedgerException {
            ......
            ......

         if (zookeeper != null) {
            zookeeper.close();
        }

        if (isBookkeeperManaged) {
            try {
                BookKeeper bkFactory = bookkeeperFactory.get();    // this create a new bookkeeper using closed zk.
                if (bkFactory != null) {
                    bkFactory.close();
                }
            } catch (BKException e) {
                throw new ManagedLedgerException(e);
            }
        }

@rdhabalia
Copy link
Contributor

@yjshen it seems issue happens when application tries to create managed-ledger-factory using self-managed-bk-constructor which constructor is not used by broker.
I have created a PR: #4580 which will fix this issue.

sijie pushed a commit that referenced this issue Jun 25, 2019
### Motivation
User can create tools on bookkeeper using ManagedLedger factory which provides [constructor](https://github.com/apache/pulsar/blob/master/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java#L121) to create ml-factory using self-managed bookkeeper (it's not used by broker).
So, in case of self-managed bk-client, ML-Factory couldn't shutdown it gracefully and we see issue: #4573

### Modification
- ML-Factory creates `DefaultBkFactory` to create self-managed bk-client and shutdowns same bk-client while closing the resource.
@sijie sijie added this to the 2.4.1 milestone Jun 25, 2019
@sijie
Copy link
Member

sijie commented Jun 25, 2019

It is fixed by #4580

@sijie sijie closed this as completed Jun 25, 2019
codelipenghui pushed a commit that referenced this issue Jun 26, 2019
### Motivation
User can create tools on bookkeeper using ManagedLedger factory which provides [constructor](https://github.com/apache/pulsar/blob/master/managed-ledger/src/main/java/org/apache/bookkeeper/mledger/impl/ManagedLedgerFactoryImpl.java#L121) to create ml-factory using self-managed bookkeeper (it's not used by broker).
So, in case of self-managed bk-client, ML-Factory couldn't shutdown it gracefully and we see issue: #4573

### Modification
- ML-Factory creates `DefaultBkFactory` to create self-managed bk-client and shutdowns same bk-client while closing the resource.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type/bug The PR fixed a bug or issue reported a bug
Projects
None yet
Development

No branches or pull requests

3 participants