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

stack commented on HBASE-5937:
------------------------------

Looking at the patch, its looking great.

You fellas like how the new Interface looks or would you change it?  Stuff like 
isLowReplicationRollEnabled, getCoprocessorHost... these seems implementation 
specific.  And this mess:

{code}
    public void hsync() throws IOException;
    public void hflush() throws IOException;
    public void sync() throws IOException;
    public void sync(long txid) throws IOException;
{code}

... You are exposing scars HBase got following the ups and downs of HDFS 
sync....and the battle rages still.

I'd be fine w/ cleanup happening in another issue after this JIRA goes in.

appendNoSync should probably be append w/ a flag passed to append instead?

No problem doing mass rename of HLog to WAL after patch goes in in another 
issue.  Here is some review:

You have to do this cast?

{code}
+    HBaseTestingUtility.setMaxRecoveryErrorCount(((FSHLog) 
wal2).getOutputStream(), 1);
{code}

... more review to follow.
                
> Refactor HLog into an interface.
> --------------------------------
>
>                 Key: HBASE-5937
>                 URL: https://issues.apache.org/jira/browse/HBASE-5937
>             Project: HBase
>          Issue Type: Sub-task
>            Reporter: Li Pi
>            Assignee: Flavio Junqueira
>            Priority: Minor
>         Attachments: 5937-hlog-with-javadoc.txt, HBASE-5937.patch, 
> HBASE-5937.patch, HBASE-5937.patch, HBASE-5937.patch, HBASE-5937.patch, 
> org.apache.hadoop.hbase.client.TestMultiParallel-output.txt
>
>
> What the summary says. Create HLog interface. Make current implementation use 
> it.

--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira

Reply via email to