hbase-issues mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Phabricator (Commented) (JIRA)" <j...@apache.org>
Subject [jira] [Commented] (HBASE-5074) support checksums in HBase block cache
Date Wed, 01 Feb 2012 19:37:01 GMT

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

Phabricator commented on HBASE-5074:
------------------------------------

dhruba has commented on the revision "[jira] [HBASE-5074] Support checksums in HBase block
cache".

  Thanks for the excellent and detailed review Mikhail. I am making most of the changes you
proposed and will post a new patch very soon. Really appreciate your time with the huge review.

INLINE COMMENTS
  src/main/java/org/apache/hadoop/hbase/io/hfile/AbstractHFileReader.java:357 The Reader would
need to reopen a file with chesksums switched on/off if needed (on checksum failure). Hence
the filessytem object is needed here.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java:402 This is messy, because there
are 100001 places where FileSystem type is being used in HBase. This will make this patch
immensely large and difficult to merge with every new change in trunk. does this sound reasonable?
If not, I can change all mention of FileSystem to HFileSystem in a succeeding patch perhaps?
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:123 It does not need a disk
format change if we decide to make it configurable. Each disk block has a 4 byte field to
store the bytes-per-checksum. In the current code, the value that is stored in this field
is 16K.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFile.java:417 This is a code cleanup but
not related to this patch. I would like to defer this for later.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:149 I did not do this because
it makes the variable names very very long-winded. Instead, I wrote more comments to describe
each variable. Let me know if you think that this is not enough for documentation.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:441-442 The meaning of extrabytes
has not changed. It still means that we need to allocate space for the next header.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:788 I put the creation of
the checksum object in a common method. The remainder of the two methods are quite similar
but unfortunately one operates on a byte buffer while the other operates directly on the ByteBuffer.
One way to merge these two methods is to incur a buffer copy which I am trying to avoid. Also,
these two methods are very specific to how the header in the HBlockFile is laid out, so I
kept them as instance methods rather than static methods. If you feel strongly about this,
then I will be happy yo move them to a different file.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:866-870 All new files always
have checksums. But the log line was for debugging, so I will get rid of it.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:973-975 Good point. I enhanced
the javadocs where the variable onDiskChecksum is declared.
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:1520-1527 This code piece
maybe done be a different helper thread. So I am throwing RunTime exception here so that the
RegionServer shuts down if it is unable to instantiate a Checksum class. Is there something
better I can do here?
  src/main/java/org/apache/hadoop/hbase/io/hfile/HFileBlock.java:1871 I actually made this
a protected method so that I can override it in the unit test to simulate checksum failure.

REVISION DETAIL
  https://reviews.facebook.net/D1521

                
> support checksums in HBase block cache
> --------------------------------------
>
>                 Key: HBASE-5074
>                 URL: https://issues.apache.org/jira/browse/HBASE-5074
>             Project: HBase
>          Issue Type: Improvement
>          Components: regionserver
>            Reporter: dhruba borthakur
>            Assignee: dhruba borthakur
>         Attachments: D1521.1.patch, D1521.1.patch
>
>
> The current implementation of HDFS stores the data in one block file and the metadata(checksum)
in another block file. This means that every read into the HBase block cache actually consumes
two disk iops, one to the datafile and one to the checksum file. This is a major problem for
scaling HBase, because HBase is usually bottlenecked on the number of random disk iops that
the storage-hardware offers.

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

        

Mime
View raw message