[jira] [Commented] (HBASE-3954) Pluggable block index

["stack (JIRA)" <ji...@apache.org>]

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

stack commented on HBASE-3954:
------------------------------

Looks good J.

You add imports in HFileOutputFormat.java but doesn't seem like you use them?

Why this is public?

{code}
+  public String blockIndexClassName = "org.apache.hadoop.hbase.io.hfile.SimpleBlockIndex";
{code}

Why not use the define you made above?

Should we let this out of HFile?

{code}
getBlockIndexImpl
{code}

Its public.  Should it be private?


This seems dodgy doing it writing out the trailer:

{code}
+      try {
+        blockIndex = (AbstractBlockIndex)Class.forName(blockIndexClass).newInstance();
+      } catch (Exception ex) {
+        throw new IOException("", ex);
+      }

{code}

We've just written a bunch of data.  If we fail the above for whatever reason, we get an exception and will likely lose the data we just wrote.  Can this reflection be done creating the Writer and kept around for writing the trailer?  If fails on construction, at least no data will have been written.

Whats going on here?

{code}
-        this.metaIndex = BlockIndex.readIndex(Bytes.BYTES_RAWCOMPARATOR, dis,
+        assert metaIndex == null;
+        this.metaIndex = new SimpleBlockIndex().readIndex(Bytes.BYTES_RAWCOMPARATOR, dis,
             this.trailer.metaIndexCount);
{code}

Why are we referring to SimpleBlockIndex by name?

There are files missing from your patch?  SimpleBlockIndex and the Interface to implement.

Good stuff Jason.

> Pluggable block index
> ---------------------
>
>                 Key: HBASE-3954
>                 URL: https://issues.apache.org/jira/browse/HBASE-3954
>             Project: HBase
>          Issue Type: Improvement
>            Reporter: Jason Rutherglen
>         Attachments: HBASE-3954.patch
>
>
> Make a pluggable block index system.  The default implementation will be the current one.

--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira