You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@bookkeeper.apache.org by "jiraposter@reviews.apache.org (Commented) (JIRA)" <ji...@apache.org> on 2011/11/03 19:23:33 UTC
[jira] [Commented] (BOOKKEEPER-50) NullPointException at
LedgerDescriptor#cmpMasterKey
[ https://issues.apache.org/jira/browse/BOOKKEEPER-50?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13143409#comment-13143409 ]
jiraposter@reviews.apache.org commented on BOOKKEEPER-50:
---------------------------------------------------------
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2642/#review3031
-----------------------------------------------------------
Looks good. I have a few comments about spelling etc, and only one comment which may require code change (regarding zero length keys).
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/2642/#comment6774>
This comment should simply be removed.
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/2642/#comment6775>
existed is wrong tense here. Should be existing. as in, existingMasterKey :)
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/2642/#comment6778>
What if the ledger is created with a zero length key?
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
<https://reviews.apache.org/r/2642/#comment6779>
Wired should be Weird.
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
<https://reviews.apache.org/r/2642/#comment6785>
Perhaps add a FileInfo javadoc for what the format is for the file.
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java
<https://reviews.apache.org/r/2642/#comment6784>
I don't like the name _read. It's not very descriptive when compared to "read". I think it would be better to rename read to readData, and rename _read to read.
http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
<https://reviews.apache.org/r/2642/#comment6776>
javadoc exceptions don't match the signature exceptions.
- Ivan
On 2011-10-31 17:39:19, Sijie Guo wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2642/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-10-31 17:39:19)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. save master key in header part of ledger index file.
bq.
bq. so we can load master key when opening an existed ledger index file to avoid that master key is null.
bq.
bq.
bq. This addresses bug BOOKKEEPER-50.
bq. http://issues.apache.org/jira/browse/BOOKKEEPER-50
bq.
bq.
bq. Diffs
bq. -----
bq.
bq. http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 1195369
bq. http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/FileInfo.java 1195369
bq. http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java 1195369
bq. http://svn.apache.org/repos/asf/zookeeper/bookkeeper/trunk/bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieReadAfterBookieRestartTest.java PRE-CREATION
bq.
bq. Diff: https://reviews.apache.org/r/2642/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Sijie
bq.
bq.
> NullPointException at LedgerDescriptor#cmpMasterKey
> ---------------------------------------------------
>
> Key: BOOKKEEPER-50
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-50
> Project: Bookkeeper
> Issue Type: Bug
> Components: bookkeeper-server
> Affects Versions: 3.4.0
> Reporter: xulei
> Assignee: Sijie Guo
> Fix For: 4.0.0
>
> Attachments: BookieReadAfterBookieRestartTest.java, bookkeeper-50.patch, bookkeeper-50.patch
>
>
> the LedgerDescriptor will be created when it is missed in LedgerCache. NullPointException will be thrown out in the following case:
> 1. The ledger descriptor is created and cached to LedgerCache because of a readEntry operation in bookie. The ledger descriptor was created without setting master key (we don't know master key in a read request)
> 2. An addEntry is sent after 1 . since the ledger descriptor has been cached, so addEntry will use it to compare master key. then NullPointException is thrown out.
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators: https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira