[kudu] branch master updated: KUDU-2348: Pick a random replica in RemoteTablet.java

[al...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

alexey pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/kudu.git


The following commit(s) were added to refs/heads/master by this push:
     new d46c141  KUDU-2348: Pick a random replica in RemoteTablet.java
d46c141 is described below

commit d46c14179312a756620eb373fcccfb429c51cb2d
Author: 张一帆 <zh...@xiaomi.com>
AuthorDate: Tue Apr 30 17:43:50 2019 +0800

    KUDU-2348: Pick a random replica in RemoteTablet.java
    
    In RemoteTablet.java, 'getClosestServerInfo' always returns
    whichever server ends up last in the map iteration order, if
    all servers in the cluster is fully remote. This behavior
    would cause heavy load on a particular server if there are
    few servers in the cluster.
    
    This patch fixes the issue by generating a random non-negative
    integer(randomInt) in RemoteTablet initializer and choose server
    based on randomInt. The randomInt variable is static so the
    choice is same across multiple RemoteTablet instances.
    
    Change-Id: I3d70e45d4c9532bb32223c1dddd0936b4ff8fd99
    Reviewed-on: http://gerrit.cloudera.org:8080/12158
    Tested-by: Kudu Jenkins
    Reviewed-by: Alexey Serbin <as...@cloudera.com>
---
 .../java/org/apache/kudu/client/RemoteTablet.java  | 43 +++++++++++++---------
 1 file changed, 25 insertions(+), 18 deletions(-)

diff --git a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
index bcfa488..74d8b9d 100644
--- a/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
+++ b/java/kudu-client/src/main/java/org/apache/kudu/client/RemoteTablet.java
@@ -24,6 +24,7 @@ import java.util.Collections;
 import java.util.HashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Random;
 import java.util.concurrent.atomic.AtomicReference;
 import javax.annotation.Nullable;
 import javax.annotation.concurrent.GuardedBy;
@@ -56,6 +57,7 @@ import org.apache.kudu.consensus.Metadata;
 public class RemoteTablet implements Comparable<RemoteTablet> {
 
   private static final Logger LOG = LoggerFactory.getLogger(RemoteTablet.class);
+  private static final int randomInt = new Random().nextInt(Integer.MAX_VALUE);
 
   private final String tableId;
   private final String tabletId;
@@ -186,35 +188,40 @@ public class RemoteTablet implements Comparable<RemoteTablet> {
    */
   @Nullable
   ServerInfo getClosestServerInfo(String location) {
-    // TODO(KUDU-2348) this doesn't return a random server, but rather returns
-    // 1. whichever local server's hashcode places it first among local servers,
-    //    if there is a local server, or
-    // 2. whichever server in the same location has a hashcode that places it
-    //    first among servers in the same location, if there is a server in the
+    // This method returns
+    // 1. a randomly picked server among local servers, if there is a local server, or
+    // 2. a randomly picked server in the same location, if there is a server in the
     //    same location, or, finally,
-    // 3. whichever server's hashcode places it last.
-    // That might be the same "random" choice across all clients, which is not
-    // so good. Unfortunately, the client depends on this method returning the
-    // same tablet server given the same state. See
-    // testGetReplicaSelectedServerInfoDeterminism in TestRemoteTablet.java.
+    // 3. a randomly picked server among all tablet servers.
     // TODO(wdberkeley): Eventually, the client might use the hierarchical
     // structure of a location to determine proximity.
     synchronized (tabletServers) {
-      ServerInfo last = null;
-      ServerInfo lastInSameLocation = null;
+      ServerInfo result = null;
+      List<ServerInfo> localServers = new ArrayList();
+      List<ServerInfo> serversInSameLocation = new ArrayList();
+      int randomIndex = randomInt % tabletServers.size();
+      int index = 0;
       for (ServerInfo e : tabletServers.values()) {
-        last = e;
         if (e.isLocal()) {
-          return e;
+          localServers.add(e);
         }
         if (e.inSameLocation(location)) {
-          lastInSameLocation = e;
+          serversInSameLocation.add(e);
         }
+        if (index == randomIndex) {
+          result = e;
+        }
+        index++;
+      }
+      if (!localServers.isEmpty()) {
+        randomIndex = randomInt % localServers.size();
+        return localServers.get(randomIndex);
       }
-      if (lastInSameLocation != null) {
-        return lastInSameLocation;
+      if (!serversInSameLocation.isEmpty()) {
+        randomIndex = randomInt % serversInSameLocation.size();
+        return serversInSameLocation.get(randomIndex);
       }
-      return last;
+      return result;
     }
   }