Apache
/
hbase
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

HBASE-15087 Fix hbase-common findbugs complaints

This commit is contained in:
stack 2016-01-12 00:30:29 -08:00
parent 8ee9158b54
commit 83c506d9d4
11 changed files with 100 additions and 21 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

33
hbase-common/src/main/java/org/apache/hadoop/hbase/CellComparator.java
View File

@ -171,9 +171,14 @@ public class CellComparator implements Comparator<Cell>, Serializable {
right.getFamilyArray(), right.getFamilyOffset(), right.getFamilyLength());
}
if (right instanceof ByteBufferedCell) {
return -(ByteBufferUtils.compareTo(((ByteBufferedCell) right).getFamilyByteBuffer(),
((ByteBufferedCell) right).getFamilyPosition(), right.getFamilyLength(),
left.getFamilyArray(), left.getFamilyOffset(), left.getFamilyLength()));
// Notice how we flip the order of the compare here. We used to negate the return value but
// see what FindBugs says
// http://findbugs.sourceforge.net/bugDescriptions.html#RV_NEGATING_RESULT_OF_COMPARETO
// It suggest flipping the order to get same effect and 'safer'.
return ByteBufferUtils.compareTo(
left.getFamilyArray(), left.getFamilyOffset(), left.getFamilyLength(),
((ByteBufferedCell)right).getFamilyByteBuffer(),
((ByteBufferedCell)right).getFamilyPosition(), right.getFamilyLength());
}
return Bytes.compareTo(left.getFamilyArray(), left.getFamilyOffset(), left.getFamilyLength(),
right.getFamilyArray(), right.getFamilyOffset(), right.getFamilyLength());
@ -210,10 +215,14 @@ public class CellComparator implements Comparator<Cell>, Serializable {
right.getQualifierArray(), right.getQualifierOffset(), right.getQualifierLength());
}
if (right instanceof ByteBufferedCell) {
return -(ByteBufferUtils.compareTo(((ByteBufferedCell) right).getQualifierByteBuffer(),
((ByteBufferedCell) right).getQualifierPosition(),
right.getQualifierLength(), left.getQualifierArray(), left.getQualifierOffset(),
left.getQualifierLength()));
// Notice how we flip the order of the compare here. We used to negate the return value but
// see what FindBugs says
// http://findbugs.sourceforge.net/bugDescriptions.html#RV_NEGATING_RESULT_OF_COMPARETO
// It suggest flipping the order to get same effect and 'safer'.
return ByteBufferUtils.compareTo(left.getQualifierArray(),
left.getQualifierOffset(), left.getQualifierLength(),
((ByteBufferedCell)right).getQualifierByteBuffer(),
((ByteBufferedCell)right).getQualifierPosition(), right.getQualifierLength());
}
return Bytes.compareTo(left.getQualifierArray(), left.getQualifierOffset(),
left.getQualifierLength(), right.getQualifierArray(), right.getQualifierOffset(),
@ -331,9 +340,13 @@ public class CellComparator implements Comparator<Cell>, Serializable {
right.getRowArray(), right.getRowOffset(), right.getRowLength());
}
if (right instanceof ByteBufferedCell) {
return -(ByteBufferUtils.compareTo(((ByteBufferedCell) right).getRowByteBuffer(),
((ByteBufferedCell) right).getRowPosition(), right.getRowLength(),
left.getRowArray(), left.getRowOffset(), left.getRowLength()));
// Notice how we flip the order of the compare here. We used to negate the return value but
// see what FindBugs says
// http://findbugs.sourceforge.net/bugDescriptions.html#RV_NEGATING_RESULT_OF_COMPARETO
// It suggest flipping the order to get same effect and 'safer'.
return ByteBufferUtils.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
((ByteBufferedCell)right).getRowByteBuffer(),
((ByteBufferedCell)right).getRowPosition(), right.getRowLength());
}
return Bytes.compareTo(left.getRowArray(), left.getRowOffset(), left.getRowLength(),
right.getRowArray(), right.getRowOffset(), right.getRowLength());

2
hbase-common/src/main/java/org/apache/hadoop/hbase/CellUtil.java
View File

@ -819,6 +819,8 @@ public final class CellUtil {
}
@Override
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="IT_NO_SUCH_ELEMENT",
justification="Intentional")
public Tag next() {
return null;
}

14
hbase-common/src/main/java/org/apache/hadoop/hbase/JitterScheduledThreadPoolExecutorImpl.java
View File

@ -92,6 +92,19 @@ public class JitterScheduledThreadPoolExecutorImpl extends ScheduledThreadPoolEx
return wrapped.compareTo(o);
}
@Override
public boolean equals(Object obj) {
if (obj == this) {
return true;
}
return obj instanceof Delayed? compareTo((Delayed)obj) == 0: false;
}
@Override
public int hashCode() {
return this.wrapped.hashCode();
}
@Override
public void run() {
wrapped.run();
@ -123,5 +136,4 @@ public class JitterScheduledThreadPoolExecutorImpl extends ScheduledThreadPoolEx
return wrapped.get(timeout, unit);
}
}
}

4
hbase-common/src/main/java/org/apache/hadoop/hbase/OffheapKeyValue.java
View File

@ -32,8 +32,8 @@ import org.apache.hadoop.hbase.util.ClassSize;
* memory.
*/
@InterfaceAudience.Private
public class OffheapKeyValue extends ByteBufferedCell implements HeapSize, Cloneable,
SettableSequenceId, Streamable {
public class OffheapKeyValue extends ByteBufferedCell
implements HeapSize, SettableSequenceId, Streamable {
protected final ByteBuffer buf;
protected final int offset;

4
hbase-common/src/main/java/org/apache/hadoop/hbase/ProcedureInfo.java
View File

@ -35,7 +35,7 @@ import org.apache.hadoop.hbase.util.NonceKey;
*/
@InterfaceAudience.Public
@InterfaceStability.Evolving
public class ProcedureInfo {
public class ProcedureInfo implements Cloneable {
private final long procId;
private final String procName;
private final String procOwner;
@ -72,6 +72,8 @@ public class ProcedureInfo {
this.result = result;
}
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="CN_IDIOM_NO_SUPER_CALL",
justification="Intentional; calling super class clone doesn't make sense here.")
public ProcedureInfo clone() {
return new ProcedureInfo(
procId, procName, procOwner, procState, parentId, exception, lastUpdate, startTime, result);

10
hbase-common/src/main/java/org/apache/hadoop/hbase/types/CopyOnWriteArrayMap.java
View File

@ -27,6 +27,7 @@ import java.util.Comparator;
import java.util.Iterator;
import java.util.Map;
import java.util.NavigableSet;
import java.util.NoSuchElementException;
import java.util.Set;
import java.util.SortedSet;
import java.util.concurrent.ConcurrentNavigableMap;
@ -693,8 +694,10 @@ public class CopyOnWriteArrayMap<K, V> extends AbstractMap<K, V>
}
@Override
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="EQ_ALWAYS_FALSE",
justification="Intentional")
public boolean equals(Object o) {
return false;
return false; // FindBugs: Causes EQ_ALWAYS_FALSE. Suppressed.
}
@Override
@ -771,7 +774,12 @@ public class CopyOnWriteArrayMap<K, V> extends AbstractMap<K, V>
}
@Override
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="IT_NO_SUCH_ELEMENT",
justification="Intentional")
public Entry<K, V> next() {
if (!hasNext()) {
throw new NoSuchElementException();
}
return holder.entries[index++];
}

32
hbase-common/src/main/java/org/apache/hadoop/hbase/util/ByteBufferUtils.java
View File

@ -38,6 +38,7 @@ import sun.nio.ch.DirectBuffer;
* Utility functions for working with byte buffers, such as reading/writing
* variable-length long numbers.
*/
@SuppressWarnings("restriction")
@InterfaceAudience.Public
@InterfaceStability.Evolving
public final class ByteBufferUtils {
@ -616,6 +617,33 @@ public final class ByteBufferUtils {
return compareTo(buf1, o1, l1, buf2, o2, l2) == 0;
}
public static int compareTo(byte [] buf1, int o1, int l1, ByteBuffer buf2, int o2, int l2) {
// This method is nearly same as the compareTo that follows but hard sharing code given
// byte array and bytebuffer types and this is a hot code path
if (UNSAFE_UNALIGNED) {
long offset2Adj;
Object refObj2 = null;
if (buf2.isDirect()) {
offset2Adj = o2 + ((DirectBuffer)buf2).address();
} else {
offset2Adj = o2 + buf2.arrayOffset() + UnsafeAccess.BYTE_ARRAY_BASE_OFFSET;
refObj2 = buf2.array();
}
return compareToUnsafe(buf1, o1 + UnsafeAccess.BYTE_ARRAY_BASE_OFFSET, l1,
refObj2, offset2Adj, l2);
}
int end1 = o1 + l1;
int end2 = o2 + l2;
for (int i = o1, j = o2; i < end1 && j < end2; i++, j++) {
int a = buf1[i] & 0xFF;
int b = buf2.get(i) & 0xFF;
if (a != b) {
return a - b;
}
}
return l1 - l2;
}
public static int compareTo(ByteBuffer buf1, int o1, int l1, byte[] buf2, int o2, int l2) {
if (UNSAFE_UNALIGNED) {
long offset1Adj;
@ -626,8 +654,8 @@ public final class ByteBufferUtils {
offset1Adj = o1 + buf1.arrayOffset() + UnsafeAccess.BYTE_ARRAY_BASE_OFFSET;
refObj1 = buf1.array();
}
return compareToUnsafe(refObj1, offset1Adj, l1, buf2, o2
+ UnsafeAccess.BYTE_ARRAY_BASE_OFFSET, l2);
return compareToUnsafe(refObj1, offset1Adj, l1,
buf2, o2 + UnsafeAccess.BYTE_ARRAY_BASE_OFFSET, l2);
}
int end1 = o1 + l1;
int end2 = o2 + l2;

4
hbase-common/src/main/java/org/apache/hadoop/hbase/util/DNS.java
View File

@ -25,6 +25,8 @@ import org.apache.hadoop.hbase.classification.InterfaceAudience;
* Wrapper around Hadoop's DNS class to hide reflection.
*/
@InterfaceAudience.Private
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="REC_CATCH_EXCEPTION",
justification="If exception, presume HAS_NEW_DNS_GET_DEFAULT_HOST_API false")
public final class DNS {
private static boolean HAS_NEW_DNS_GET_DEFAULT_HOST_API;
private static Method GET_DEFAULT_HOST_METHOD;
@ -35,7 +37,7 @@ public final class DNS {
.getMethod("getDefaultHost", String.class, String.class, boolean.class);
HAS_NEW_DNS_GET_DEFAULT_HOST_API = true;
} catch (Exception e) {
HAS_NEW_DNS_GET_DEFAULT_HOST_API = false;
HAS_NEW_DNS_GET_DEFAULT_HOST_API = false; // FindBugs: Causes REC_CATCH_EXCEPTION. Suppressed
}
}

4
hbase-common/src/main/java/org/apache/hadoop/hbase/util/DynamicClassLoader.java
View File

@ -99,7 +99,9 @@ public class DynamicClassLoader extends ClassLoaderBase {
}
}
private void initTempDir(final Configuration conf) {
// FindBugs: Making synchronized to avoid IS2_INCONSISTENT_SYNC complaints about
// remoteDirFs and jarModifiedTime being part synchronized protected.
private synchronized void initTempDir(final Configuration conf) {
jarModifiedTime = new HashMap<String, Long>();
String localDirPath = conf.get(
LOCAL_DIR_KEY, DEFAULT_LOCAL_DIR) + DYNAMIC_JARS_DIR;

5
hbase-common/src/main/java/org/apache/hadoop/hbase/util/UnsafeAccess.java
View File

@ -34,6 +34,8 @@ import sun.nio.ch.DirectBuffer;
@InterfaceAudience.Private
@InterfaceStability.Evolving
@edu.umd.cs.findbugs.annotations.SuppressWarnings(value="REC_CATCH_EXCEPTION",
justification="If exception, presume unaligned")
public final class UnsafeAccess {
private static final Log LOG = LogFactory.getLog(UnsafeAccess.class);
@ -51,7 +53,6 @@ public final class UnsafeAccess {
// copyMemory method. A limit is imposed to allow for safepoint polling
// during a large copy
static final long UNSAFE_COPY_THRESHOLD = 1024L * 1024L;
static {
theUnsafe = (Unsafe) AccessController.doPrivileged(new PrivilegedAction<Object>() {
@Override
@ -76,7 +77,7 @@ public final class UnsafeAccess {
m.setAccessible(true);
unaligned = (boolean) m.invoke(null);
} catch (Exception e) {
unaligned = false;
unaligned = false; // FindBugs: Causes REC_CATCH_EXCEPTION. Suppressed.
}
} else{
BYTE_ARRAY_BASE_OFFSET = -1;

9
hbase-server/src/test/java/org/apache/hadoop/hbase/util/TestByteBufferUtils.java
View File

@ -387,6 +387,15 @@ public class TestByteBufferUtils {
assertTrue(ByteBufferUtils.compareTo(bb1, 0, bb1.remaining(), bb2, 0, bb2.remaining()) < 0);
bb2.put(6, (byte) 4);
assertTrue(ByteBufferUtils.compareTo(bb1, 0, bb1.remaining(), bb2, 0, bb2.remaining()) > 0);
// Assert reverse comparing BB and bytearray works.
ByteBuffer bb3 = ByteBuffer.allocate(135);
fillBB(bb3, (byte)0);
byte[] b3 = new byte[135];
fillArray(b3, (byte)1);
int result = ByteBufferUtils.compareTo(b3, 0, b3.length, bb3, 0, bb3.remaining());
assertTrue(result > 0);
result = ByteBufferUtils.compareTo(bb3, 0, bb3.remaining(), b3, 0, b3.length);
assertTrue(result < 0);
}
private static void fillBB(ByteBuffer bb, byte b) {