v1

hbase-client/src/main/java/org/apache/hadoop/hbase/client/HConnectable.java

@@ -1,50 +0,0 @@
-/**
- * Copyright The Apache Software Foundation
- *
- * Licensed to the Apache Software Foundation (ASF) under one
- * or more contributor license agreements.  See the NOTICE file
- * distributed with this work for additional information
- * regarding copyright ownership.  The ASF licenses this file
- * to you under the Apache License, Version 2.0 (the
- * "License"); you may not use this file except in compliance
- * with the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.hadoop.hbase.client;
-
-import java.io.IOException;
-
-import org.apache.hadoop.conf.Configuration;
-import org.apache.hadoop.hbase.classification.InterfaceAudience;
-
-/**
- * This class makes it convenient for one to execute a command in the context
- * of a {@link HConnection} instance based on the given {@link Configuration}.
- *
- * <p>
- * If you find yourself wanting to use a {@link HConnection} for a relatively
- * short duration of time, and do not want to deal with the hassle of creating
- * and cleaning up that resource, then you should consider using this
- * convenience class.
- *
- * @param <T>
- *          the return type of the {@link HConnectable#connect(HConnection)}
- *          method.
- */
-@InterfaceAudience.Private
-public abstract class HConnectable<T> {
-  protected Configuration conf;
-
-  protected HConnectable(Configuration conf) {
-    this.conf = conf;
-  }
-
-  public abstract T connect(HConnection connection) throws IOException;
-}

hbase-client/src/main/java/org/apache/hadoop/hbase/client/MetricsConnection.java

@@ -105,7 +105,7 @@ public class MetricsConnection {
   }
 
   @VisibleForTesting
-  protected final class CallTracker {
+  protected static final class CallTracker {
     private final String name;
     @VisibleForTesting final Timer callTimer;
     @VisibleForTesting final Histogram reqHist;

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RetriesExhaustedException.java

@@ -85,8 +85,7 @@ public class RetriesExhaustedException extends IOException {
   public RetriesExhaustedException(final int numRetries,
                                    final List<ThrowableWithExtraContext> exceptions) {
     super(getMessage(numRetries, exceptions),
-        (exceptions != null && !exceptions.isEmpty() ?
-            exceptions.get(exceptions.size() - 1).t : null));
+      exceptions.isEmpty()? null: exceptions.get(exceptions.size() - 1).t);
   }
 
   private static String getMessage(String callableVitals, int numTries,

hbase-client/src/main/java/org/apache/hadoop/hbase/client/RetryingCallerInterceptorFactory.java

@@ -59,6 +59,8 @@ class RetryingCallerInterceptorFactory {
    *         {@link RetryingCallerInterceptor} object according to the
    *         configuration.
    */
+  @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="REC_CATCH_EXCEPTION",
+      justification="Convert thrown exception to unchecked")
   public RetryingCallerInterceptor build() {
     RetryingCallerInterceptor ret = NO_OP_INTERCEPTOR;
     if (failFast) {

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/BinaryComparator.java

@@ -53,7 +53,7 @@ public class BinaryComparator extends ByteArrayComparable {
 
   @Override
   public int compareTo(ByteBuffer value, int offset, int length) {
-    return -(ByteBufferUtils.compareTo(value, offset, length, this.value, 0, this.value.length));
+    return ByteBufferUtils.compareTo(this.value, 0, this.value.length, value, offset, length);
   }
 
   /**

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/BinaryPrefixComparator.java

@@ -58,7 +58,7 @@ public class BinaryPrefixComparator extends ByteArrayComparable {
     if (this.value.length <= length) {
       length = this.value.length;
     }
-    return -(ByteBufferUtils.compareTo(value, offset, length, this.value, 0, this.value.length));
+    return ByteBufferUtils.compareTo(this.value, 0, this.value.length, value, offset, length);
   }
 
   /**

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/FuzzyRowFilter.java

@@ -189,12 +189,8 @@ public class FuzzyRowFilter extends FilterBase {
                 @Override
                 public int compare(Pair<byte[], Pair<byte[], byte[]>> o1,
                     Pair<byte[], Pair<byte[], byte[]>> o2) {
-                  int compare = Bytes.compareTo(o1.getFirst(), o2.getFirst());
-                  if (!isReversed()) {
-                    return compare;
-                  } else {
-                    return -compare;
-                  }
+                  return isReversed()? Bytes.compareTo(o2.getFirst(), o1.getFirst()):
+                    Bytes.compareTo(o1.getFirst(), o2.getFirst());
                 }
               });
     }

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/LongComparator.java

@@ -89,8 +89,6 @@ public class LongComparator extends ByteArrayComparable {
      */
     boolean areSerializedFieldsEqual(LongComparator other) {
         if (other == this) return true;
-        if (!(other instanceof LongComparator)) return false;
-
         return super.areSerializedFieldsEqual(other);
     }
 }

hbase-client/src/main/java/org/apache/hadoop/hbase/filter/MultiRowRangeFilter.java

@@ -503,6 +503,8 @@ public class MultiRowRangeFilter extends FilterBase {
     }
 
     @Override
+    @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="EQ_COMPARETO_USE_OBJECT_EQUALS",
+      justification="This compareTo is not of this Object, but of referenced RowRange")
     public int compareTo(RowRange other) {
       return Bytes.compareTo(this.startRow, other.startRow);
     }

hbase-client/src/main/java/org/apache/hadoop/hbase/ipc/RpcClientImpl.java

@@ -383,7 +383,7 @@ public class RpcClientImpl extends AbstractRpcClient {
       }
     }
 
-    private UserInformation getUserInfo(UserGroupInformation ugi) {
+    private synchronized UserInformation getUserInfo(UserGroupInformation ugi) {
       if (ugi == null || authMethod == AuthMethod.DIGEST) {
         // Don't send user for token auth
         return null;
@@ -804,7 +804,9 @@ public class RpcClientImpl extends AbstractRpcClient {
       byte [] preamble = new byte [rpcHeaderLen + 2];
       System.arraycopy(HConstants.RPC_HEADER, 0, preamble, 0, rpcHeaderLen);
       preamble[rpcHeaderLen] = HConstants.RPC_CURRENT_VERSION;
-      preamble[rpcHeaderLen + 1] = authMethod.code;
+      synchronized (this) {
+        preamble[rpcHeaderLen + 1] = authMethod.code;
+      }
       outStream.write(preamble);
       outStream.flush();
     }
@@ -880,6 +882,8 @@ public class RpcClientImpl extends AbstractRpcClient {
      * threads.
      * @see #readResponse()
      */
+    @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="IS2_INCONSISTENT_SYNC",
+        justification="Findbugs is misinterpreting locking missing fact that this.outLock is held")
     private void writeRequest(Call call, final int priority, Span span) throws IOException {
       RequestHeader.Builder builder = RequestHeader.newBuilder();
       builder.setCallId(call.id);
@@ -913,8 +917,8 @@ public class RpcClientImpl extends AbstractRpcClient {
         checkIsOpen(); // Now we're checking that it didn't became idle in between.
 
         try {
-          call.callStats.setRequestSizeBytes(
-              IPCUtil.write(this.out, header, call.param, cellBlock));
+          call.callStats.setRequestSizeBytes(IPCUtil.write(this.out, header, call.param,
+              cellBlock));
         } catch (IOException e) {
           // We set the value inside the synchronized block, this way the next in line
           //  won't even try to write. Otherwise we might miss a call in the calls map?
@@ -932,14 +936,20 @@ public class RpcClientImpl extends AbstractRpcClient {
 
       // We added a call, and may be started the connection close. In both cases, we
       //  need to notify the reader.
-      synchronized (this) {
-        notifyAll();
-      }
+      doNotify();
 
       // Now that we notified, we can rethrow the exception if any. Otherwise we're good.
       if (writeException != null) throw writeException;
     }
 
+    @edu.umd.cs.findbugs.annotations.SuppressWarnings(value="NN_NAKED_NOTIFY",
+        justification="Presume notifyAll is because we are closing/shutting down")
+    private synchronized void doNotify() {
+      // Make a separate method so can do synchronize and add findbugs annotation; only one
+      // annotation at at time in source 1.7.
+      notifyAll(); // Findbugs: NN_NAKED_NOTIFY
+    }
+
     /* Receive a response.
      * Because only one receiver, so no synchronization on in.
      */

hbase-client/src/main/java/org/apache/hadoop/hbase/protobuf/RequestConverter.java

@@ -463,7 +463,7 @@ public final class RequestConverter {
     builder.setScan(ProtobufUtil.toScan(scan));
     builder.setClientHandlesPartials(true);
     builder.setClientHandlesHeartbeats(true);
-    builder.setTrackScanMetrics(scan != null && scan.isScanMetricsEnabled());
+    builder.setTrackScanMetrics(scan.isScanMetricsEnabled());
     return builder.build();
   }