[
https://issues.apache.org/jira/browse/BOOKKEEPER-106?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13150527#comment-13150527
]
[email protected] commented on BOOKKEEPER-106:
----------------------------------------------------------
bq. On 2011-11-15 13:53:03, fpj wrote:
bq. > Ivan, It is not very easy to review this patch because there is
reformatting mixed with the fix itself. My understanding is that the fix itself
is essentially the implementation of getNewBookie and the changes to call it.
Could you please describe the changes in slightly more detail so that it helps
the review?
The refactoring in BookKeeperAdmin is part of the fix. The sequence of
callbacks and the exact detail of what they do had to be restructured. I
updated the description of the review to explain the change. Its quite complex.
For BookieRecoveryTest.java, everything below line 430 is new. I had to move
the test from org.apache.bookkeeper.test to org.apache.bookkeeper.client to
allow it to access the metadata from the ledger handle.
bq. On 2011-11-15 13:53:03, fpj wrote:
bq. >
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java,
line 600
bq. > <https://reviews.apache.org/r/2806/diff/1/?file=57480#file57480line600>
bq. >
bq. > Is the formatting of this block correct? It doesn't look like on
review board but I need to check on a text editor.
What looks wrong with it? availableBookies is lined up with the start of the
argument list which is in line with Sun conventions
(http://www.oracle.com/technetwork/java/codeconventions-136091.html#248).
There's a few superfluous spaces (rb marks red), which i need to remove.
bq. On 2011-11-15 13:53:03, fpj wrote:
bq. >
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java,
line 605
bq. > <https://reviews.apache.org/r/2806/diff/1/?file=57482#file57482line605>
bq. >
bq. > Is this todo to the lh.close that is commented out?
This is something I need to open another JIRA for. Basically, if you recover a
bookie, all ledgers recovered will have their zookeeper metadata changed. If
another process is currently writing to the ledger, when they try to close the
handle, they will get a KeeperException.BadVersion because the version of the
ledger znode has changed. It's not a correctness issue, but more of something
that could be annoying for users.
- Ivan
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/2806/#review3253
-----------------------------------------------------------
On 2011-11-11 16:05:38, Ivan Kelly wrote:
bq.
bq. -----------------------------------------------------------
bq. This is an automatically generated e-mail. To reply, visit:
bq. https://reviews.apache.org/r/2806/
bq. -----------------------------------------------------------
bq.
bq. (Updated 2011-11-11 16:05:38)
bq.
bq.
bq. Review request for bookkeeper.
bq.
bq.
bq. Summary
bq. -------
bq.
bq. As the summary says, if you don't specify a destBookie when doing
recoveryBookieData, it will select at random from the available bookie list. It
doesn't take care to select a bookie which is not is the ledgers ensemble.
bq.
bq. Attached patch fixes this problem and also cleans up BookKeeperAdmin a
little. There were a lot of nested callbacks nested in more callbacks etc. I've
tries to unnest a little.
bq.
bq. Ledger metadata is now updated on each fragment. It used to be updated
when the whole ledger was recovered, but only specified a single possible
ledger. This was incorrect as it could lead to underreplication. There can be
contention in the writes to the ledger data, but just retry as the previously
successful write should have updated the stat, and they will be writing from
the same ledger handle.
bq.
bq. I've also added a couple of tests and some test framework stuff to verify
that entries are all replicated.
bq.
bq.
bq. This addresses bug BOOKKEEPER-106.
bq. https://issues.apache.org/jira/browse/BOOKKEEPER-106
bq.
bq.
bq. Diffs
bq. -----
bq.
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/BookKeeperAdmin.java
b3eb5b9
bq.
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
f1b3ad9
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
PRE-CREATION
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BaseTestCase.java
6bac569
bq.
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieRecoveryTest.java
ac54d9a
bq.
bq. Diff: https://reviews.apache.org/r/2806/diff
bq.
bq.
bq. Testing
bq. -------
bq.
bq.
bq. Thanks,
bq.
bq. Ivan
bq.
bq.
> recoveryBookieData can select a recovery bookie which is already in the
> ledgers ensemble
> ----------------------------------------------------------------------------------------
>
> Key: BOOKKEEPER-106
> URL: https://issues.apache.org/jira/browse/BOOKKEEPER-106
> Project: Bookkeeper
> Issue Type: Bug
> Reporter: Ivan Kelly
> Assignee: Ivan Kelly
> Priority: Blocker
> Fix For: 4.0.0
>
> Attachments: BOOKKEEPER-106.diff
>
>
> As the summary says, if you don't specify a destBookie when doing
> recoveryBookieData, it will select at random from the available bookie list.
> It doesn't take care to select a bookie which is not is the ledgers ensemble.
--
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
