From 039bc73571d2cc89378749573dfeec74c247b0b9 Mon Sep 17 00:00:00 2001 From: BELUGA BEHR Date: Wed, 4 Apr 2018 14:12:19 -0700 Subject: [PATCH] HBASE-19488 Move to using Apache commons CollectionUtils Signed-off-by: Apekshit Sharma --- .../hadoop/hbase/client/RowMutations.java | 5 +- .../org/apache/hadoop/hbase/util/Bytes.java | 1 + .../hadoop/hbase/util/CollectionUtils.java | 80 ------------------- .../replication/ZKReplicationPeerStorage.java | 5 +- .../ZKReplicationQueueStorage.java | 24 ++++-- .../hadoop/hbase/regionserver/HRegion.java | 2 +- .../hbase/regionserver/RSRpcServices.java | 2 +- .../hbase/regionserver/StoreScanner.java | 6 +- .../hbase/regionserver/wal/FSWALEntry.java | 2 +- 9 files changed, 30 insertions(+), 97 deletions(-) diff --git a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java index 1eb3151ae0e..4b426cf10e8 100644 --- a/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java +++ b/hbase-client/src/main/java/org/apache/hadoop/hbase/client/RowMutations.java @@ -22,8 +22,9 @@ import java.util.ArrayList; import java.util.Arrays; import java.util.Collections; import java.util.List; + +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.yetus.audience.InterfaceAudience; /** @@ -47,7 +48,7 @@ public class RowMutations implements Row { */ public static RowMutations of(List mutations) throws IOException { if (CollectionUtils.isEmpty(mutations)) { - throw new IllegalArgumentException("Can't instantiate a RowMutations by empty list"); + throw new IllegalArgumentException("Cannot instantiate a RowMutations by empty list"); } return new RowMutations(mutations.get(0).getRow(), mutations.size()) .add(mutations); diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java index b7912fd3f56..a315fd2b39e 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/Bytes.java @@ -38,6 +38,7 @@ import java.util.Comparator; import java.util.Iterator; import java.util.List; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.KeyValue; diff --git a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/CollectionUtils.java b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/CollectionUtils.java index 8bbb6f1e386..bfe41d8f387 100644 --- a/hbase-common/src/main/java/org/apache/hadoop/hbase/util/CollectionUtils.java +++ b/hbase-common/src/main/java/org/apache/hadoop/hbase/util/CollectionUtils.java @@ -19,10 +19,6 @@ package org.apache.hadoop.hbase.util; import java.io.IOException; -import java.util.ArrayList; -import java.util.Collection; -import java.util.Collections; -import java.util.List; import java.util.concurrent.ConcurrentMap; import java.util.function.Supplier; @@ -34,82 +30,6 @@ import org.apache.yetus.audience.InterfaceAudience; @InterfaceAudience.Private public class CollectionUtils { - private static final List EMPTY_LIST = Collections.unmodifiableList(new ArrayList<>(0)); - - - @SuppressWarnings("unchecked") - public static Collection nullSafe(Collection in) { - if (in == null) { - return (Collection)EMPTY_LIST; - } - return in; - } - - /************************ size ************************************/ - - public static int nullSafeSize(Collection collection) { - if (collection == null) { - return 0; - } - return collection.size(); - } - - public static boolean nullSafeSameSize(Collection a, Collection b) { - return nullSafeSize(a) == nullSafeSize(b); - } - - /*************************** empty ****************************************/ - - public static boolean isEmpty(Collection collection) { - return collection == null || collection.isEmpty(); - } - - public static boolean notEmpty(Collection collection) { - return !isEmpty(collection); - } - - /************************ first/last **************************/ - - public static T getFirst(Collection collection) { - if (CollectionUtils.isEmpty(collection)) { - return null; - } - for (T t : collection) { - return t; - } - return null; - } - - /** - * @param list any list - * @return -1 if list is empty, otherwise the max index - */ - public static int getLastIndex(List list){ - if(isEmpty(list)){ - return -1; - } - return list.size() - 1; - } - - /** - * @param list - * @param index the index in question - * @return true if it is the last index or if list is empty and -1 is passed for the index param - */ - public static boolean isLastIndex(List list, int index){ - return index == getLastIndex(list); - } - - public static T getLast(List list) { - if (isEmpty(list)) { - return null; - } - return list.get(list.size() - 1); - } - - public static List nullToEmpty(List list) { - return list != null ? list : Collections.emptyList(); - } /** * In HBASE-16648 we found that ConcurrentHashMap.get is much faster than computeIfAbsent if the * value already exists. Notice that the implementation does not guarantee that the supplier will diff --git a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationPeerStorage.java b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationPeerStorage.java index a53500a8905..bbe65498738 100644 --- a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationPeerStorage.java +++ b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationPeerStorage.java @@ -18,11 +18,11 @@ package org.apache.hadoop.hbase.replication; import java.util.Arrays; +import java.util.Collections; import java.util.List; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.hbase.client.replication.ReplicationPeerConfigUtil; import org.apache.hadoop.hbase.exceptions.DeserializationException; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZKUtil.ZKUtilOp; import org.apache.hadoop.hbase.zookeeper.ZKWatcher; @@ -131,7 +131,8 @@ public class ZKReplicationPeerStorage extends ZKReplicationStorageBase @Override public List listPeerIds() throws ReplicationException { try { - return CollectionUtils.nullToEmpty(ZKUtil.listChildrenNoWatch(zookeeper, peersZNode)); + List children = ZKUtil.listChildrenNoWatch(zookeeper, peersZNode); + return children != null ? children : Collections.emptyList(); } catch (KeeperException e) { throw new ReplicationException("Cannot get the list of peers", e); } diff --git a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationQueueStorage.java b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationQueueStorage.java index 1a4ccc1a205..da96c65fcdb 100644 --- a/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationQueueStorage.java +++ b/hbase-replication/src/main/java/org/apache/hadoop/hbase/replication/ZKReplicationQueueStorage.java @@ -18,7 +18,6 @@ package org.apache.hadoop.hbase.replication; import static java.util.stream.Collectors.toList; -import static org.apache.hadoop.hbase.util.CollectionUtils.nullToEmpty; import java.util.ArrayList; import java.util.Collections; @@ -27,13 +26,13 @@ import java.util.List; import java.util.Set; import java.util.SortedSet; import java.util.TreeSet; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; import org.apache.hadoop.hbase.HConstants; import org.apache.hadoop.hbase.ServerName; import org.apache.hadoop.hbase.exceptions.DeserializationException; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.util.Pair; import org.apache.hadoop.hbase.zookeeper.ZKUtil; import org.apache.hadoop.hbase.zookeeper.ZKUtil.ZKUtilOp; @@ -258,8 +257,11 @@ class ZKReplicationQueueStorage extends ZKReplicationStorageBase } private List getListOfReplicators0() throws KeeperException { - return nullToEmpty(ZKUtil.listChildrenNoWatch(zookeeper, queuesZNode)).stream() - .map(ServerName::parseServerName).collect(toList()); + List children = ZKUtil.listChildrenNoWatch(zookeeper, queuesZNode); + if (children == null) { + children = Collections.emptyList(); + } + return children.stream().map(ServerName::parseServerName).collect(toList()); } @Override @@ -273,7 +275,9 @@ class ZKReplicationQueueStorage extends ZKReplicationStorageBase private List getWALsInQueue0(ServerName serverName, String queueId) throws KeeperException { - return nullToEmpty(ZKUtil.listChildrenNoWatch(zookeeper, getQueueNode(serverName, queueId))); + List children = ZKUtil.listChildrenNoWatch(zookeeper, getQueueNode(serverName, + queueId)); + return children != null ? children : Collections.emptyList(); } @Override @@ -289,7 +293,8 @@ class ZKReplicationQueueStorage extends ZKReplicationStorageBase } private List getAllQueues0(ServerName serverName) throws KeeperException { - return nullToEmpty(ZKUtil.listChildrenNoWatch(zookeeper, getRsNode(serverName))); + List children = ZKUtil.listChildrenNoWatch(zookeeper, getRsNode(serverName)); + return children != null ? children : Collections.emptyList(); } @Override @@ -409,7 +414,8 @@ class ZKReplicationQueueStorage extends ZKReplicationStorageBase } private List getAllPeersFromHFileRefsQueue0() throws KeeperException { - return nullToEmpty(ZKUtil.listChildrenNoWatch(zookeeper, hfileRefsZNode)); + List children = ZKUtil.listChildrenNoWatch(zookeeper, hfileRefsZNode); + return children != null ? children : Collections.emptyList(); } @Override @@ -423,7 +429,9 @@ class ZKReplicationQueueStorage extends ZKReplicationStorageBase } private List getReplicableHFiles0(String peerId) throws KeeperException { - return nullToEmpty(ZKUtil.listChildrenNoWatch(this.zookeeper, getHFileRefsPeerNode(peerId))); + List children = ZKUtil.listChildrenNoWatch(this.zookeeper, + getHFileRefsPeerNode(peerId)); + return children != null ? children : Collections.emptyList(); } @Override diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java index 5f0af2eab99..23284be209e 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/HRegion.java @@ -70,6 +70,7 @@ import java.util.concurrent.locks.Lock; import java.util.concurrent.locks.ReadWriteLock; import java.util.concurrent.locks.ReentrantReadWriteLock; import java.util.function.Function; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.FileStatus; import org.apache.hadoop.fs.FileSystem; @@ -155,7 +156,6 @@ import org.apache.hadoop.hbase.trace.TraceUtil; import org.apache.hadoop.hbase.util.Bytes; import org.apache.hadoop.hbase.util.CancelableProgressable; import org.apache.hadoop.hbase.util.ClassSize; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.util.CompressionTest; import org.apache.hadoop.hbase.util.EncryptionTest; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java index 2c2d3cccfaf..264b2c1e56a 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/RSRpcServices.java @@ -42,6 +42,7 @@ import java.util.concurrent.TimeUnit; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.atomic.AtomicLong; import java.util.concurrent.atomic.LongAdder; +import org.apache.commons.collections.CollectionUtils; import org.apache.commons.lang3.mutable.MutableObject; import org.apache.hadoop.conf.Configuration; import org.apache.hadoop.fs.Path; @@ -125,7 +126,6 @@ import org.apache.hadoop.hbase.security.User; import org.apache.hadoop.hbase.security.access.AccessChecker; import org.apache.hadoop.hbase.security.access.Permission; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.util.DNS; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.hadoop.hbase.util.Pair; diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java index 2bc1e015083..2973e57819d 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/StoreScanner.java @@ -27,6 +27,7 @@ import java.util.OptionalInt; import java.util.concurrent.CountDownLatch; import java.util.concurrent.locks.ReentrantLock; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellUtil; @@ -45,13 +46,14 @@ import org.apache.hadoop.hbase.regionserver.handler.ParallelSeekHandler; import org.apache.hadoop.hbase.regionserver.querymatcher.CompactionScanQueryMatcher; import org.apache.hadoop.hbase.regionserver.querymatcher.ScanQueryMatcher; import org.apache.hadoop.hbase.regionserver.querymatcher.UserScanQueryMatcher; -import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.util.EnvironmentEdgeManager; import org.apache.yetus.audience.InterfaceAudience; + import org.slf4j.Logger; import org.slf4j.LoggerFactory; + import org.apache.hbase.thirdparty.com.google.common.annotations.VisibleForTesting; +import org.apache.hbase.thirdparty.com.google.common.base.Preconditions; /** * Scanner scans both the memstore and the Store. Coalesce KeyValue stream into List<KeyValue> diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java index 6edeaed0807..4e69b3959e4 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/regionserver/wal/FSWALEntry.java @@ -25,6 +25,7 @@ import java.util.List; import java.util.Set; import java.util.TreeSet; +import org.apache.commons.collections.CollectionUtils; import org.apache.hadoop.hbase.Cell; import org.apache.hadoop.hbase.CellComparator; import org.apache.hadoop.hbase.CellUtil; @@ -32,7 +33,6 @@ import org.apache.hadoop.hbase.PrivateCellUtil; import org.apache.hadoop.hbase.client.RegionInfo; import org.apache.hadoop.hbase.regionserver.MultiVersionConcurrencyControl; import org.apache.hadoop.hbase.util.Bytes; -import org.apache.hadoop.hbase.util.CollectionUtils; import org.apache.hadoop.hbase.wal.WAL.Entry; import org.apache.hadoop.hbase.wal.WALEdit; import org.apache.hadoop.hbase.wal.WALKeyImpl;