hbase-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From st...@apache.org
Subject hbase git commit: HBASE-13634 Avoid unsafe reference equality checks to EMPTY byte[] Add comment
Date Mon, 11 May 2015 17:56:19 GMT
Repository: hbase
Updated Branches:
  refs/heads/master 671ae8f15 -> fa6dc9c44


HBASE-13634 Avoid unsafe reference equality checks to EMPTY byte[]
Add comment

Signed-off-by: stack <stack@apache.org>


Project: http://git-wip-us.apache.org/repos/asf/hbase/repo
Commit: http://git-wip-us.apache.org/repos/asf/hbase/commit/fa6dc9c4
Tree: http://git-wip-us.apache.org/repos/asf/hbase/tree/fa6dc9c4
Diff: http://git-wip-us.apache.org/repos/asf/hbase/diff/fa6dc9c4

Branch: refs/heads/master
Commit: fa6dc9c44e28aa608d5204db814e8442b95eb125
Parents: 671ae8f
Author: Lars Francke <lars.francke@gmail.com>
Authored: Wed May 6 18:13:46 2015 +0200
Committer: stack <stack@apache.org>
Committed: Mon May 11 10:55:48 2015 -0700

----------------------------------------------------------------------
 .../hbase/regionserver/ScanQueryMatcher.java    |  5 ++--
 .../regionserver/StripeMultiFileWriter.java     | 22 +++++++++------
 .../regionserver/StripeStoreFileManager.java    | 29 ++++++++++++++------
 .../hbase/util/RegionSplitCalculator.java       |  1 +
 4 files changed, 38 insertions(+), 19 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/hbase/blob/fa6dc9c4/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
index 86390da..e67e292 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/ScanQueryMatcher.java
@@ -20,6 +20,7 @@
 package org.apache.hadoop.hbase.regionserver;
 
 import java.io.IOException;
+import java.util.Arrays;
 import java.util.NavigableSet;
 
 import org.apache.hadoop.hbase.KeyValue.Type;
@@ -474,7 +475,7 @@ public class ScanQueryMatcher {
     // dropDeletesFromRow is leq current kv, we start dropping deletes and reset
     // dropDeletesFromRow; thus the 2nd "if" starts to apply.
     if ((dropDeletesFromRow != null)
-        && ((dropDeletesFromRow == HConstants.EMPTY_START_ROW)
+        && (Arrays.equals(dropDeletesFromRow, HConstants.EMPTY_START_ROW)
           || (Bytes.compareTo(row, offset, length,
               dropDeletesFromRow, 0, dropDeletesFromRow.length) >= 0))) {
       retainDeletesInOutput = false;
@@ -484,7 +485,7 @@ public class ScanQueryMatcher {
     // drop-deletes range. When dropDeletesToRow is leq current kv, we stop dropping deletes,
     // and reset dropDeletesToRow so that we don't do any more compares.
     if ((dropDeletesFromRow == null)
-        && (dropDeletesToRow != null) && (dropDeletesToRow != HConstants.EMPTY_END_ROW)
+        && (dropDeletesToRow != null) && !Arrays.equals(dropDeletesToRow,
HConstants.EMPTY_END_ROW)
         && (Bytes.compareTo(row, offset, length,
             dropDeletesToRow, 0, dropDeletesToRow.length) >= 0)) {
       retainDeletesInOutput = true;

http://git-wip-us.apache.org/repos/asf/hbase/blob/fa6dc9c4/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
index 26c9470..99f4a6b 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeMultiFileWriter.java
@@ -19,6 +19,7 @@ package org.apache.hadoop.hbase.regionserver;
 
 import java.io.IOException;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
 
@@ -116,7 +117,7 @@ public abstract class StripeMultiFileWriter implements Compactor.CellSink
{
    */
   protected void sanityCheckLeft(
       byte[] left, Cell cell) throws IOException {
-    if (StripeStoreFileManager.OPEN_KEY != left &&
+    if (!Arrays.equals(StripeStoreFileManager.OPEN_KEY, left) &&
         comparator.compareRows(cell, left, 0, left.length) < 0) {
       String error = "The first row is lower than the left boundary of [" + Bytes.toString(left)
           + "]: [" + Bytes.toString(cell.getRowArray(), cell.getRowOffset(), cell.getRowLength())
@@ -132,7 +133,7 @@ public abstract class StripeMultiFileWriter implements Compactor.CellSink
{
    */
   protected void sanityCheckRight(
       byte[] right, Cell cell) throws IOException {
-    if (StripeStoreFileManager.OPEN_KEY != right &&
+    if (!Arrays.equals(StripeStoreFileManager.OPEN_KEY, right) &&
         comparator.compareRows(cell, right, 0, right.length) >= 0) {
       String error = "The last row is higher or equal than the right boundary of ["
           + Bytes.toString(right) + "]: ["
@@ -178,10 +179,14 @@ public abstract class StripeMultiFileWriter implements Compactor.CellSink
{
       // must match some target boundaries, let's find them.
       assert  (majorRangeFrom == null) == (majorRangeTo == null);
       if (majorRangeFrom != null) {
-        majorRangeFromIndex = (majorRangeFrom == StripeStoreFileManager.OPEN_KEY) ? 0
-          : Collections.binarySearch(this.boundaries, majorRangeFrom, Bytes.BYTES_COMPARATOR);
-        majorRangeToIndex = (majorRangeTo == StripeStoreFileManager.OPEN_KEY) ? boundaries.size()
-          : Collections.binarySearch(this.boundaries, majorRangeTo, Bytes.BYTES_COMPARATOR);
+        majorRangeFromIndex = Arrays.equals(majorRangeFrom, StripeStoreFileManager.OPEN_KEY)
+                                ? 0
+                                : Collections.binarySearch(boundaries, majorRangeFrom,
+                                                           Bytes.BYTES_COMPARATOR);
+        majorRangeToIndex = Arrays.equals(majorRangeTo, StripeStoreFileManager.OPEN_KEY)
+                              ? boundaries.size()
+                              : Collections.binarySearch(boundaries, majorRangeTo,
+                                                         Bytes.BYTES_COMPARATOR);
         if (this.majorRangeFromIndex < 0 || this.majorRangeToIndex < 0) {
           throw new IOException("Major range does not match writer boundaries: [" +
               Bytes.toString(majorRangeFrom) + "] [" + Bytes.toString(majorRangeTo) + "];
from "
@@ -204,9 +209,8 @@ public abstract class StripeMultiFileWriter implements Compactor.CellSink
{
     }
 
     private boolean isCellAfterCurrentWriter(Cell cell) {
-      return ((currentWriterEndKey != StripeStoreFileManager.OPEN_KEY) &&
-            (comparator.compareRows(cell,
-                currentWriterEndKey, 0, currentWriterEndKey.length) >= 0));
+      return !Arrays.equals(currentWriterEndKey, StripeStoreFileManager.OPEN_KEY) &&
+            (comparator.compareRows(cell, currentWriterEndKey, 0, currentWriterEndKey.length)
>= 0);
     }
 
     @Override

http://git-wip-us.apache.org/repos/asf/hbase/blob/fa6dc9c4/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
index 0f03018..4481a5f 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StripeStoreFileManager.java
@@ -505,6 +505,7 @@ public class StripeStoreFileManager
    * Checks whether the key is invalid (e.g. from an L0 file, or non-stripe-compacted files).
    */
   private static final boolean isInvalid(byte[] key) {
+    // No need to use Arrays.equals because INVALID_KEY is null
     return key == INVALID_KEY;
   }
 
@@ -536,8 +537,10 @@ public class StripeStoreFileManager
    * Finds the stripe index for the stripe containing a row provided externally for get/scan.
    */
   private final int findStripeForRow(byte[] row, boolean isStart) {
-    if (isStart && row == HConstants.EMPTY_START_ROW) return 0;
-    if (!isStart && row == HConstants.EMPTY_END_ROW) return state.stripeFiles.size()
- 1;
+    if (isStart && Arrays.equals(row, HConstants.EMPTY_START_ROW)) return 0;
+    if (!isStart && Arrays.equals(row, HConstants.EMPTY_END_ROW)) {
+      return state.stripeFiles.size() - 1;
+    }
     // If there's an exact match below, a stripe ends at "row". Stripe right boundary is
     // exclusive, so that means the row is in the next stripe; thus, we need to add one to
index.
     // If there's no match, the return value of binarySearch is (-(insertion point) - 1),
where
@@ -559,15 +562,25 @@ public class StripeStoreFileManager
 
 
   private byte[] startOf(StoreFile sf) {
-    byte[] result = this.fileStarts.get(sf);
-    return result == null ? sf.getMetadataValue(STRIPE_START_KEY)
-        : (result == INVALID_KEY_IN_MAP ? INVALID_KEY : result);
+    byte[] result = fileStarts.get(sf);
+
+    // result and INVALID_KEY_IN_MAP are compared _only_ by reference on purpose here as
the latter
+    // serves only as a marker and is not to be confused with other empty byte arrays.
+    // See Javadoc of INVALID_KEY_IN_MAP for more information
+    return (result == null)
+             ? sf.getMetadataValue(STRIPE_START_KEY)
+             : result == INVALID_KEY_IN_MAP ? INVALID_KEY : result;
   }
 
   private byte[] endOf(StoreFile sf) {
-    byte[] result = this.fileEnds.get(sf);
-    return result == null ? sf.getMetadataValue(STRIPE_END_KEY)
-        : (result == INVALID_KEY_IN_MAP ? INVALID_KEY : result);
+    byte[] result = fileEnds.get(sf);
+
+    // result and INVALID_KEY_IN_MAP are compared _only_ by reference on purpose here as
the latter
+    // serves only as a marker and is not to be confused with other empty byte arrays.
+    // See Javadoc of INVALID_KEY_IN_MAP for more information
+    return (result == null)
+             ? sf.getMetadataValue(STRIPE_END_KEY)
+             : result == INVALID_KEY_IN_MAP ? INVALID_KEY : result;
   }
 
   /**

http://git-wip-us.apache.org/repos/asf/hbase/blob/fa6dc9c4/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
----------------------------------------------------------------------
diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
index b904b43..eeef1ae 100644
--- a/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
+++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/util/RegionSplitCalculator.java
@@ -115,6 +115,7 @@ public class RegionSplitCalculator<R extends KeyRange> {
     byte[] start = range.getStartKey();
     byte[] end = specialEndKey(range);
 
+    // No need to use Arrays.equals because ENDKEY is null
     if (end != ENDKEY && Bytes.compareTo(start, end) > 0) {
       // don't allow backwards edges
       LOG.debug("attempted to add backwards edge: " + Bytes.toString(start)


Mime
View raw message