Repository: hbase Updated Branches: refs/heads/master 5bfc5745a -> e83ac38d6
HBASE-20078 MultiByteBuff : bug in reading primitives when individual buffers are too small. Project: http://git-wip-us.apache.org/repos/asf/hbase/repo Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/e83ac38d Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/e83ac38d Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/e83ac38d Branch: refs/heads/master Commit: e83ac38d68baa5f142afbe2e2f30e23161943eb0 Parents: 5bfc574 Author: anoopsamjohn <anoopsamj...@gmail.com> Authored: Wed Mar 14 12:07:57 2018 +0530 Committer: anoopsamjohn <anoopsamj...@gmail.com> Committed: Wed Mar 14 12:07:57 2018 +0530 ---------------------------------------------------------------------- .../apache/hadoop/hbase/nio/MultiByteBuff.java | 84 +++----------------- .../hadoop/hbase/nio/TestMultiByteBuff.java | 31 ++++++++ 2 files changed, 40 insertions(+), 75 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/hbase/blob/e83ac38d/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java index 847e2eb..97f5141 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/nio/MultiByteBuff.java @@ -260,16 +260,10 @@ public class MultiByteBuff extends ByteBuff { // means cur item is the last one and we wont be able to read a int. Throw exception throw new BufferUnderflowException(); } - ByteBuffer nextItem = items[itemIndex + 1]; - // Get available bytes from this item and remaining from next int l = 0; - for (int i = offsetInItem; i < item.capacity(); i++) { - l <<= 8; - l ^= ByteBufferUtils.toByte(item, i) & 0xFF; - } - for (int i = 0; i < Bytes.SIZEOF_INT - remainingLen; i++) { + for (int i = 0; i < Bytes.SIZEOF_INT; i++) { l <<= 8; - l ^= ByteBufferUtils.toByte(nextItem, i) & 0xFF; + l ^= get(index + i) & 0xFF; } return l; } @@ -310,16 +304,10 @@ public class MultiByteBuff extends ByteBuff { // means cur item is the last one and we wont be able to read a long. Throw exception throw new BufferUnderflowException(); } - ByteBuffer nextItem = items[itemIndex + 1]; - // Get available bytes from this item and remaining from next long l = 0; - for (int i = offsetInItem; i < item.capacity(); i++) { - l <<= 8; - l ^= ByteBufferUtils.toByte(item, i) & 0xFF; - } - for (int i = 0; i < Bytes.SIZEOF_LONG - remainingLen; i++) { + for (int i = 0; i < Bytes.SIZEOF_LONG; i++) { l <<= 8; - l ^= ByteBufferUtils.toByte(nextItem, i) & 0xFF; + l ^= get(index + i) & 0xFF; } return l; } @@ -339,28 +327,7 @@ public class MultiByteBuff extends ByteBuff { } else { itemIndex = getItemIndex(index); } - ByteBuffer item = items[itemIndex]; - int offsetInItem = index - this.itemBeginPos[itemIndex]; - int remainingLen = item.limit() - offsetInItem; - if (remainingLen >= Bytes.SIZEOF_LONG) { - return ByteBufferUtils.toLong(item, offsetInItem); - } - if (items.length - 1 == itemIndex) { - // means cur item is the last one and we wont be able to read a long. Throw exception - throw new BufferUnderflowException(); - } - ByteBuffer nextItem = items[itemIndex + 1]; - // Get available bytes from this item and remaining from next - long l = 0; - for (int i = offsetInItem; i < item.capacity(); i++) { - l <<= 8; - l ^= ByteBufferUtils.toByte(item, i) & 0xFF; - } - for (int i = 0; i < Bytes.SIZEOF_LONG - remainingLen; i++) { - l <<= 8; - l ^= ByteBufferUtils.toByte(nextItem, i) & 0xFF; - } - return l; + return getLong(index, itemIndex); } @Override @@ -513,15 +480,6 @@ public class MultiByteBuff extends ByteBuff { if (remaining >= Bytes.SIZEOF_SHORT) { return this.curItem.getShort(); } - if (remaining == 0) { - if (items.length - 1 == this.curItemIndex) { - // means cur item is the last one and we wont be able to read a long. Throw exception - throw new BufferUnderflowException(); - } - this.curItemIndex++; - this.curItem = this.items[this.curItemIndex]; - return this.curItem.getShort(); - } short n = 0; n = (short) (n ^ (get() & 0xFF)); n = (short) (n << 8); @@ -540,16 +498,6 @@ public class MultiByteBuff extends ByteBuff { if (remaining >= Bytes.SIZEOF_INT) { return this.curItem.getInt(); } - if (remaining == 0) { - if (items.length - 1 == this.curItemIndex) { - // means cur item is the last one and we wont be able to read a long. Throw exception - throw new BufferUnderflowException(); - } - this.curItemIndex++; - this.curItem = this.items[this.curItemIndex]; - return this.curItem.getInt(); - } - // Get available bytes from this item and remaining from next int n = 0; for (int i = 0; i < Bytes.SIZEOF_INT; i++) { n <<= 8; @@ -570,16 +518,6 @@ public class MultiByteBuff extends ByteBuff { if (remaining >= Bytes.SIZEOF_LONG) { return this.curItem.getLong(); } - if (remaining == 0) { - if (items.length - 1 == this.curItemIndex) { - // means cur item is the last one and we wont be able to read a long. Throw exception - throw new BufferUnderflowException(); - } - this.curItemIndex++; - this.curItem = this.items[this.curItemIndex]; - return this.curItem.getLong(); - } - // Get available bytes from this item and remaining from next long l = 0; for (int i = 0; i < Bytes.SIZEOF_LONG; i++) { l <<= 8; @@ -613,8 +551,7 @@ public class MultiByteBuff extends ByteBuff { toRead); this.curItem.position(this.curItem.position() + toRead); length -= toRead; - if (length == 0) - break; + if (length == 0) break; this.curItemIndex++; this.curItem = this.items[this.curItemIndex]; offset += toRead; @@ -631,8 +568,7 @@ public class MultiByteBuff extends ByteBuff { ByteBufferUtils.copyFromBufferToArray(dst, item, sourceOffset, offset, toRead); length -= toRead; - if (length == 0) - break; + if (length == 0) break; itemIndex++; item = this.items[itemIndex]; offset += toRead; @@ -985,8 +921,7 @@ public class MultiByteBuff extends ByteBuff { ByteBufferUtils .copyFromBufferToArray(dupB, locCurItem, locCurItem.position(), offset, toRead); length -= toRead; - if (length == 0) - break; + if (length == 0) break; locCurItemIndex++; locCurItem = this.items[locCurItemIndex]; offset += toRead; @@ -1090,8 +1025,7 @@ public class MultiByteBuff extends ByteBuff { // doing more reads from Channel. Only this much there for now. break; } else { - if (this.curItemIndex >= this.limitedItemIndex) - break; + if (this.curItemIndex >= this.limitedItemIndex) break; this.curItemIndex++; this.curItem = this.items[this.curItemIndex]; } http://git-wip-us.apache.org/repos/asf/hbase/blob/e83ac38d/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java ---------------------------------------------------------------------- diff --git a/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java b/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java index 95c088e..84cf7a4 100644 --- a/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java +++ b/hbase-common/src/test/java/org/apache/hadoop/hbase/nio/TestMultiByteBuff.java @@ -426,4 +426,35 @@ public class TestMultiByteBuff { mbb1.get(); // Now we have reached the limit assertFalse(mbb1.hasRemaining()); } + + @Test + public void testGetPrimitivesWithSmallIndividualBBs() { + short s = 45; + int i = 2345; + long l = 75681526L; + ByteBuffer bb = ByteBuffer.allocate(14); + bb.putShort(s); + bb.putInt(i); + bb.putLong(l); + + ByteBuffer bb1 = ((ByteBuffer) bb.duplicate().position(0).limit(1)).slice(); + ByteBuffer bb2 = ((ByteBuffer) bb.duplicate().position(1).limit(3)).slice(); + ByteBuffer bb3 = ((ByteBuffer) bb.duplicate().position(3).limit(5)).slice(); + ByteBuffer bb4 = ((ByteBuffer) bb.duplicate().position(5).limit(11)).slice(); + ByteBuffer bb5 = ((ByteBuffer) bb.duplicate().position(11).limit(12)).slice(); + ByteBuffer bb6 = ((ByteBuffer) bb.duplicate().position(12).limit(14)).slice(); + MultiByteBuff mbb = new MultiByteBuff(bb1, bb2, bb3, bb4, bb5, bb6); + assertEquals(s, mbb.getShortAfterPosition(0)); + assertEquals(i, mbb.getIntAfterPosition(2)); + assertEquals(l, mbb.getLongAfterPosition(6)); + + assertEquals(s, mbb.getShort(0)); + assertEquals(i, mbb.getInt(2)); + assertEquals(l, mbb.getLong(6)); + + mbb.position(0); + assertEquals(s, mbb.getShort()); + assertEquals(i, mbb.getInt()); + assertEquals(l, mbb.getLong()); + } }
