Skip to content

Issue 1927: fix NoNodeException in LocalBookeeper#2082

Merged
ivankelly merged 4 commits intoapache:masterfrom
zhaohaidao:master
Jun 6, 2019
Merged

Issue 1927: fix NoNodeException in LocalBookeeper#2082
ivankelly merged 4 commits intoapache:masterfrom
zhaohaidao:master

Conversation

@zhaohaidao
Copy link
Contributor

Motivation

It addresses #1927 where it provides a conf check at the beginning of startLocalBookiesInternal function.

Changes

  • Add a conf check at the beginning of startLocalBookiesInternal function where non-default zkLedgersRootPath is not allowed.

cc @sijie

@zhaohaidao
Copy link
Contributor Author

Failed replication tests finished with timeout in my laptop(macOS).
It seems that it blocked at test env setup step according the test log.
My test command is as followed.
mvn clean install -DskipTests; mvn -pl bookkeeper-server clean test -Dtest=BookieAutoRecoveryTest#testEmptyLedgerLosesQuorumEventually

Did I execute the right command ? Or I missed something. The test log is attached as followed. Thanks

org.apache.bookkeeper.replication.BookieAutoRecoveryTest-output.txt

@sijie
Copy link
Member

sijie commented May 5, 2019

run bookkeeper-server replication tests

@zhaohaidao
Copy link
Contributor Author

Looks like my commit affects case-BookieAutoRecoveryTest#testEmptyLedgerLosesQuorumEventually.
However, this case failed with timeout in my laptop(macOS). Should I do some preparation for test in macOS?

Failed replication tests finished with timeout in my laptop(macOS).
It seems that it blocked at test env setup step according the test log.
My test command is as followed.
mvn clean install -DskipTests; mvn -pl bookkeeper-server clean test -Dtest=BookieAutoRecoveryTest#testEmptyLedgerLosesQuorumEventually

Did I execute the right command ? Or I missed something. The test log is attached as followed. Thanks

org.apache.bookkeeper.replication.BookieAutoRecoveryTest-output.txt

@ivankelly
Copy link
Contributor

This doesn't address #1927. The problem in #1927 is that if you have something like /streaming/bookkeeper/ledgers as your root path, localbookie will fail to start. With this fix, it still fails to start. To address the issue, you need to create the path on boot.

@zhaohaidao
Copy link
Contributor Author

@ivankelly Thanks for your advice.
My fix logic is that it is simpler to constrain the input range than dealing with arbitrary input.
Although this fix will still fail with /streaming/bookkeeper/ledgers as the root path, the error message means it is an illegal parameter error rather than a runtime error like NoNodeException.
Then if users see the error message, they will update their zkLedgerRootPath.

@ivankelly
Copy link
Contributor

@zhaohaidao the fix to permit the paths is one line, using ZkUtils#createFullPathOptimistic

@zhaohaidao
Copy link
Contributor Author

@ivankelly Thanks for your reminder. I submitted a fix as per your suggestion. Please have a look at it.

@sijie
Copy link
Member

sijie commented May 20, 2019

run bookkeeper-server replication tests

@sijie
Copy link
Member

sijie commented May 21, 2019

@eolivelli can you please take a look at this PR again?

@sijie
Copy link
Member

sijie commented May 27, 2019

ping @eolivelli

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants