Repository: hbase
Updated Branches:
  refs/heads/branch-2 a9d9fa35a -> 7bd39250e


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/7bd39250
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/7bd39250
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/7bd39250

Branch: refs/heads/branch-2
Commit: 7bd39250e71f84d0d04c1f09538f5b30ec3207d1
Parents: a9d9fa3
Author: anoopsamjohn <anoopsamj...@gmail.com>
Authored: Wed Mar 14 12:07:57 2018 +0530
Committer: anoopsamjohn <anoopsamj...@gmail.com>
Committed: Wed Mar 14 12:10:22 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/7bd39250/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/7bd39250/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());
+  }
 }

Reply via email to