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 8d4ea4d..adb37c8 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 @@ -27,16 +27,8 @@ import java.net.BindException; import java.net.InetSocketAddress; import java.net.UnknownHostException; import java.nio.ByteBuffer; -import java.util.ArrayList; -import java.util.Collections; -import java.util.HashMap; -import java.util.Iterator; -import java.util.List; -import java.util.Map; +import java.util.*; import java.util.Map.Entry; -import java.util.NavigableMap; -import java.util.Set; -import java.util.TreeSet; import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentMap; import java.util.concurrent.atomic.AtomicLong; @@ -870,6 +862,29 @@ public class RSRpcServices implements HBaseRPCErrorHandler, return cellsToReturn; } + + /** + * A comparator to compare ClientProtos.Action + * We can't use Bytes.CompareTo() because each time we call ByteString.toByteArray() + * A byte array copy is made. We can't accept that + */ + static final Comparator actionCompartor = new Comparator() { + @Override + public int compare(ClientProtos.Action o1, ClientProtos.Action o2) { + ByteString b1 = o1.getMutation().getRow(); + ByteString b2 = o2.getMutation().getRow(); + int minLength = Math.min(b1.size(), b2.size()); + for(int i = 0; i < minLength; i++) { + int a = (b1.byteAt(i) & 0xff); + int b = (b2.byteAt(i) & 0xff); + if (a != b) { + return a - b; + } + } + return b1.size() - b2.size(); + } + }; + /** * Execute a list of Put/Delete mutations. * @@ -883,6 +898,17 @@ public class RSRpcServices implements HBaseRPCErrorHandler, Mutation[] mArray = new Mutation[mutations.size()]; long before = EnvironmentEdgeManager.currentTime(); boolean batchContainsPuts = false, batchContainsDelete = false; + /** + * HBASE-17924 + * We need to sort ClientProtos.Action to improve lock efficiency + * The reason of sorting mutations instead of mArray is that + * we need to make sure mutation and Action are one-to-one mapping, + * since we need to set the right result or exception to + * the corresponding action. If we sort mArray instead of mutations + * Those mapping can be disorganized. + * + */ + Collections.sort(mutations, actionCompartor); try { int i = 0; for (ClientProtos.Action action: mutations) { @@ -1951,6 +1977,11 @@ public class RSRpcServices implements HBaseRPCErrorHandler, walEntries.add(walEntry); } if(edits!=null && !edits.isEmpty()) { + /** + * HBASE-17924 + * We need to sort edits to improve lock efficiency + */ + Collections.sort(edits); long replaySeqId = (entry.getKey().hasOrigSequenceNumber()) ? entry.getKey().getOrigSequenceNumber() : entry.getKey().getLogSequenceNumber(); OperationStatus[] result = doReplayBatchOp(region, edits, replaySeqId); diff --git a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java index 77c2d1c..9b5c4bc 100644 --- a/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java +++ b/hbase-server/src/main/java/org/apache/hadoop/hbase/wal/WALSplitter.java @@ -2279,7 +2279,7 @@ public class WALSplitter { } /** A struct used by getMutationsFromWALEntry */ - public static class MutationReplay { + public static class MutationReplay implements Comparable { public MutationReplay(MutationType type, Mutation mutation, long nonceGroup, long nonce) { this.type = type; this.mutation = mutation; @@ -2295,6 +2295,20 @@ public class WALSplitter { public final Mutation mutation; public final long nonceGroup; public final long nonce; + + @Override + public int compareTo(final MutationReplay d) { + return this.mutation.compareTo(d.mutation); + } + + @Override + public boolean equals(Object obj) { + if(!(obj instanceof MutationReplay)) { + return false; + } else { + return this.mutation.equals(((MutationReplay) obj).mutation); + } + } } /**