[
https://issues.apache.org/jira/browse/HBASE-1818?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12753327#action_12753327
]
ryan rawson commented on HBASE-1818:
------------------------------------
Here are some comments from the svn commit:
if (this.comparator.compare(this.lastKeyBuffer, this.lastKeyOffset,
- this.lastKeyLength, key, offset, length) > 0) {
+ this.lastKeyLength, key, offset, length) >= 0) {
throw new IOException("Added a key not lexically larger than" +
" previous key=" + Bytes.toString(key, offset, length) +
", lastkey=" + Bytes.toString(this.lastKeyBuffer,
this.lastKeyOffset,
This change has the _Exact opposite_ effect from the intent. The intent (as I
construct it) is to allow duplicate keys in a HFile, yet by adding >= you are
explicitly requiring the next key to be larger, but NOT equal to the previous
key. This has to stay as is.
+ if (buf == null)
+ return null;
This is pointless, we'll NPE a few lines later anyways. Don't add checking code
that doesnt have any benefits - the caller may not be interpret a null return
as 'error' in this case, did you check that?
- // Toss the header. May have to remove later due to performance.
- buf.compact();
- buf.limit(buf.limit() - METABLOCKMAGIC.length);
- buf.rewind();
I am not super happy with this change... it doesnt affect performance, but it
does reduce code simplicity, and leaves open room for errors further on, and
requires everyone to realize that the buf has this hidden header which isnt
part of the public API of HFile.
> HFile code review and refinement
> --------------------------------
>
> Key: HBASE-1818
> URL: https://issues.apache.org/jira/browse/HBASE-1818
> Project: Hadoop HBase
> Issue Type: Improvement
> Components: io
> Affects Versions: 0.20.0
> Reporter: Schubert Zhang
> Assignee: Schubert Zhang
> Priority: Minor
> Fix For: 0.21.0
>
> Attachments: HFile-v1.patch, HFile-v2.patch
>
>
> HFile is a good mimic of Google's SSTable file format. And we want HFile to
> become a common file format of hadoop in the near future.
> We will review the code of HFile and record the comments here, and then
> provide fixed patch after the review.
--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.
