[
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
