[ 
https://issues.apache.org/jira/browse/BOOKKEEPER-135?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13195544#comment-13195544
 ] 

[email protected] commented on BOOKKEEPER-135:
----------------------------------------------------------



bq.  On 2012-01-28 11:00:01, fpj wrote:
bq.  > 
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java, 
line 370
bq.  > <https://reviews.apache.org/r/3642/diff/1/?file=70865#file70865line370>
bq.  >
bq.  >     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.
bq.  >

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);
                        }
                    }


bq.  On 2012-01-28 11:00:01, fpj wrote:
bq.  > 
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java,
 line 293
bq.  > <https://reviews.apache.org/r/3642/diff/1/?file=70863#file70863line293>
bq.  >
bq.  >     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:
bq.  
bq.  -----------------------------------------------------------
bq.  This is an automatically generated e-mail. To reply, visit:
bq.  https://reviews.apache.org/r/3642/
bq.  -----------------------------------------------------------
bq.  
bq.  (Updated 2012-01-26 19:26:03)
bq.  
bq.  
bq.  Review request for bookkeeper.
bq.  
bq.  
bq.  Summary
bq.  -------
bq.  
bq.  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.
bq.  
bq.  
bq.  This addresses bug BOOKKEEPER-135.
bq.      https://issues.apache.org/jira/browse/BOOKKEEPER-135
bq.  
bq.  
bq.  Diffs
bq.  -----
bq.  
bq.    bookkeeper-server/src/main/java/org/apache/bookkeeper/bookie/Bookie.java 
cb3bb26 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/LedgerRecoveryOp.java
 4625bbb 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/PendingReadOp.java 
29070eb 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/client/ReadLastConfirmedOp.java
 43e999d 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieClient.java 
4466ce3 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieProtocol.java 
bc1cfb0 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/BookieServer.java 
beab5e8 
bq.    
bookkeeper-server/src/main/java/org/apache/bookkeeper/proto/PerChannelBookieClient.java
 2fa79bb 
bq.    
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/BookieRecoveryTest.java
 8526db5 
bq.    
bookkeeper-server/src/test/java/org/apache/bookkeeper/client/TestFencing.java 
015e4e4 
bq.    
bookkeeper-server/src/test/java/org/apache/bookkeeper/test/BookieClientTest.java
 99d6ef0 
bq.  
bq.  Diff: https://reviews.apache.org/r/3642/diff
bq.  
bq.  
bq.  Testing
bq.  -------
bq.  
bq.  
bq.  Thanks,
bq.  
bq.  Ivan
bq.  
bq.


                
> Fencing does not check the ledger masterPasswd
> ----------------------------------------------
>
>                 Key: BOOKKEEPER-135
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-135
>             Project: Bookkeeper
>          Issue Type: Bug
>            Reporter: Ivan Kelly
>            Assignee: Ivan Kelly
>             Fix For: 4.1.0
>
>         Attachments: BOOKKEEPER-135.diff
>
>
> 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 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

        

Reply via email to