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

Sijie Guo commented on BOOKKEEPER-199:
--------------------------------------

thanks Rakesh, it is good work. 

just some comments over your patch.

1) in BookieWatcher, if a bookie server is restarted (maybe by admin guys) from 
R-O mode, seems there is no logic to remove a bookie from R-O mode to write 
mode. do I miss something?

2) it would be better to put "R-O" as a constant define, maybe put it in 
BookieProtocol

3) I don't seem anyone use "roEnsembles" defined in LedgerMetadata.java. so who 
will use it?

4) the exception is thrown when there is no writable dirs found, so the error 
message should be improved so admin guys could known what happened. BTW, I am 
not sure BadLedgerDirsException is good enough to describe such kind of issue, 
how about NoWritableLedgerDirException?
{code}
throw new Bookie.BadLedgerDirsException("Failed to create new log in the ledger 
dirs : " + list);
{code}

{code}
+        ArrayList<File> listOfDirs = new ArrayList<File>(list);
+        listOfDirs.removeAll(writeFailedDirs);
{code}
How about we keep a list of writable dirs, so we don't need to init a list and 
remove failed dirs again. 

5) in Bookie#addEntryWithRo, 
{code}
+                zk.setData(myPresenceInZK, "R-O".getBytes(), 0);
{code}
seems like it is a conditional set, it might fail if other one did changes on 
that znode to increment znode version. I am not sure is there any consideration 
when you using 0 version. why not using -1?
from my side, in future we could provide a tool for admin guys to turn a bookie 
server into R-O mode to fix some issue then turn it back to W-R mode. so the 
znode version would be changed outside bookie server, it would be better to 
using -1 matching any versions of znode.

BTW, it would be better to put a patch to review board 
(https://reviews.apache.org) if the patch is not simple, which might be 
convient for reviewing :)
                
> Provide bookie readonly mode, when journal/ledgers flushing has failed with 
> IOE
> -------------------------------------------------------------------------------
>
>                 Key: BOOKKEEPER-199
>                 URL: https://issues.apache.org/jira/browse/BOOKKEEPER-199
>             Project: Bookkeeper
>          Issue Type: Sub-task
>          Components: bookkeeper-server
>    Affects Versions: 4.0.0
>            Reporter: Rakesh R
>            Assignee: Rakesh R
>            Priority: Critical
>             Fix For: 4.1.0
>
>         Attachments: BOOKKEEPER-199.patch
>
>
> Bookkeeper should change to readonly(r-o) mode when the journal/ledgers 
> flushing has failed with IOException. Later on, reject write requests on 
> server side and will accept only the read requests from the clients, because 
> even if flushing fails, the data in the bookie which has been flushed is 
> still valid.

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