> On 2012-01-30 10:06:44, Ivan Kelly wrote:
> > I'm thinking I should add something more explicit to allow the client see 
> > that the actual issue is the wrong password, maybe by throwing a different 
> > exception, or logging a very explicit error message.
> >

Sounds like a good idea, since LedgerRecoveryException is not precise enough. 
The ReadException is also mixing wrong password and read failure, right?


> On 2012-01-30 10:06:44, Ivan Kelly 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>
> >
> >     This isn't were the error would come from in the case of wrong password 
> > for fencing. Listener here is only for writing to the socket. Once you've 
> > written to the socket the callback is invoked. 
> >     
> >     For wrong password errors, we have to wait for the response from the 
> > bookie, which is handled in #messageReceived, and in this case by 
> > #handleReadResponse, which will interpret the rc as a ReadException. When 
> > LedgerRecoveryOp#readEntryComplete receives ReadException for all of the 
> > read requests, it throws a LedgerRecoveryException to the client.

Got it, seems right to me.


- fpj


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://reviews.apache.org/r/3642/#review4685
-----------------------------------------------------------


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
> 
>

Reply via email to