> On 2012-01-28 11:00:01, fpj wrote:
> > bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java,
> > line 370
> > <https://reviews.apache.org/r/3642/diff/1/?file=70865#file70865line370>
> >
> > I don't think we currently throw this exception anywhere in the code
> > (BKLedgerRecoveryException). It is nowhere else in this patch and it is not
> > present in trunk. I think we are throwing an exception here, but not the
> > the right one.
> >
My bad, I just realized that we do it here in LedgerOpenOp:
public void operationComplete(int rc, Void result) {
if (rc != BKException.Code.OK) {
cb.openComplete(BKException.Code.LedgerRecoveryException, null,
LedgerOpenOp.this.ctx);
} else {
cb.openComplete(BKException.Code.OK, lh,
LedgerOpenOp.this.ctx);
}
}
> On 2012-01-28 11:00:01, fpj wrote:
> > bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java,
> > line 293
> > <https://reviews.apache.org/r/3642/diff/1/?file=70863#file70863line293>
> >
> > Is this where we fail the openLedger? If so, by checking the method, it
> > doesn't seem to be using the right code. This is related to my other
> > comment on the exception not being the right one.
It sounds like in LedgerRecoveryOp we just pass the error code to LedgerOpenOp
and if it is not OK, we make it a LedgerRecoveryException. Consequently, it
works to return BKException.Code.BookieHandleNotAvailableException in
PerChannelBookieClient.errorOutReadKey(). The error here is not
BookieHandleNotAvailable. Should we fix this one here?
- fpj
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3642/#review4668
-----------------------------------------------------------
On 2012-01-26 19:26:03, Ivan Kelly wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://reviews.apache.org/r/3642/
> -----------------------------------------------------------
>
> (Updated 2012-01-26 19:26:03)
>
>
> Review request for bookkeeper.
>
>
> Summary
> -------
>
> When fencing, the ledger handle is not checked before the fencing is applied.
> Currently the openLedger does fail, on because it will addEntry and fail at
> that point, but by this stage, fencing has already been applied. The check
> should be earlier.
>
>
> This addresses bug BOOKKEEPER-135.
> https://issues.apache.org/jira/browse/BOOKKEEPER-135
>
>
> Diffs
> -----
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java
> cb3bb26
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
> 4625bbb
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java
> 29070eb
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java
> 43e999d
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java
> 4466ce3
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java
> bc1cfb0
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java
> beab5e8
>
> bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
> 2fa79bb
>
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
> 8526db5
>
> bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java
> 015e4e4
>
> bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java
> 99d6ef0
>
> Diff: https://reviews.apache.org/r/3642/diff
>
>
> Testing
> -------
>
>
> Thanks,
>
> Ivan
>
>
