From cd1b964bb7e3b821c72ebee434064a31daf76b2a Mon Sep 17 00:00:00 2001 From: Zach York Date: Thu, 31 Aug 2017 10:25:32 -0700 Subject: [PATCH] HBASE-18757 Fix improper bitwise & in bucketcache offset calculation This correctly casts the operand to a long to avoid negative offsets created by sign extending the integer operand. Signed-off-by: tedyu --- .../apache/hadoop/hbase/io/hfile/bucket/BucketCache.java | 4 ++-- .../hadoop/hbase/io/hfile/bucket/TestBucketCache.java | 9 +++++++++ 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java index 2a8afa74fc7..58f32234d35 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/io/hfile/bucket/BucketCache.java @@ -1285,8 +1285,8 @@ public class BucketCache implements BlockCache, HeapSize { } long offset() { // Java has no unsigned numbers - long o = ((long) offsetBase) & 0xFFFFFFFF; - o += (((long) (offset1)) & 0xFF) << 32; + long o = ((long) offsetBase) & 0xFFFFFFFFL; //This needs the L cast otherwise it will be sign extended as a negative number. + o += (((long) (offset1)) & 0xFF) << 32; //The 0xFF here does not need the L cast because it is treated as a positive int. return o << 8; } diff --git a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java index 8cd665e3bf3..5a2a51cf420 100644 --- a/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java +++ b/hbase-server/src/test/java/org/apache/hadoop/hbase/io/hfile/bucket/TestBucketCache.java @@ -388,4 +388,13 @@ public class TestBucketCache { long expectedOutput = (long) Math.floor(bucketCache.getAllocator().getTotalSize() * partitionFactor * minFactor); assertEquals(expectedOutput, bucketCache.getPartitionSize(partitionFactor)); } + + @Test + public void testOffsetProducesPositiveOutput() { + //This number is picked because it produces negative output if the values isn't ensured to be positive. + //See HBASE-18757 for more information. + long testValue = 549888460800L; + BucketCache.BucketEntry bucketEntry = new BucketCache.BucketEntry(testValue, 10, 10L, true); + assertEquals(testValue, bucketEntry.offset()); + } }