Merge pull request #209 from Netflix/NullExceptionBinaryKeys

Avoid reading binary keys when not needed
Netflix · Feb 15, 2018 · 681578e · 681578e
2 parents 287b5bb + 2a485dc
commit 681578e
Show file tree

Hide file tree

Showing 2 changed files with 30 additions and 20 deletions.
diff --git a/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/lb/TokenAwareSelection.java b/dyno-core/src/main/java/com/netflix/dyno/connectionpool/impl/lb/TokenAwareSelection.java
@@ -21,6 +21,7 @@
 import java.util.Map;
 import java.util.concurrent.ConcurrentHashMap;
 
+import com.netflix.dyno.connectionpool.exception.BadRequestException;
 import org.apache.commons.lang3.StringUtils;
 
 import com.netflix.dyno.connectionpool.BaseOperation;
@@ -86,9 +87,9 @@ public Long get(HostToken x) {
     public HostConnectionPool<CL> getPoolForOperation(BaseOperation<CL, ?> op, String hashtag) throws NoAvailableHostsException {
 
         String key = op.getStringKey();
-        byte[] binaryKey = op.getBinaryKey();
+        HostConnectionPool<CL> hostPool;
+        HostToken hToken;
 
-        HostToken hToken = null;
         if (key != null) {
         	  // If a hashtag is provided by Dynomite then we use that to create the key to hash.
             if (hashtag == null || hashtag.isEmpty()) {            
@@ -97,27 +98,33 @@ public HostConnectionPool<CL> getPoolForOperation(BaseOperation<CL, ?> op, Strin
                 String hashValue = StringUtils.substringBetween(key,Character.toString(hashtag.charAt(0)), Character.toString(hashtag.charAt(1)));
                 hToken = this.getTokenForKey(hashValue);
             }
-        }
-        else {
-        	// the key is binary 
-        	hToken = this.getTokenForKey(binaryKey);
-        }
-
 
-        HostConnectionPool<CL> hostPool = null;
-        if (hToken != null) {
+            if (hToken == null) {
+                throw new NoAvailableHostsException("Token not found for key " + key);
+            }
+
+            hostPool = tokenPools.get(hToken.getToken());;
+            if (hostPool == null) {
+                throw new NoAvailableHostsException(
+                        "Could not find host connection pool for key: " + key + ", hash: " + tokenMapper.hash(key) + " Token:" + hToken.getToken());
+            }
+        } else {
+        	// the key is binary
+            byte[] binaryKey = op.getBinaryKey();
+            hToken = this.getTokenForKey(binaryKey);
+            if (hToken == null) {
+                throw new NoAvailableHostsException("Token not found for key " + binaryKey.toString());
+            }
+
             hostPool = tokenPools.get(hToken.getToken());
-        }
-
-        if (hostPool == null && key !=null) {
-             throw new NoAvailableHostsException(
-                        "Could not find host connection pool for key: " + key + ", hash: " + tokenMapper.hash(key));
-        }
-        else if (hostPool == null) {
-            throw new NoAvailableHostsException(
-                    "Could not find host connection pool for key: " + binaryKey.toString() + ", hash: " + tokenMapper.hash(binaryKey));
+            if (hostPool == null) {
+                throw new NoAvailableHostsException(
+                        "Could not find host connection pool for key: " + binaryKey.toString() + ", hash: " + tokenMapper.hash(binaryKey) + " Token:" + getTokenForKey(binaryKey));
+            }
+
         }
         return hostPool;
+
     }
 
     @Override

diff --git a/dyno-jedis/src/main/java/com/netflix/dyno/jedis/DynoJedisClient.java b/dyno-jedis/src/main/java/com/netflix/dyno/jedis/DynoJedisClient.java
@@ -145,7 +145,10 @@ public String getStringKey() {
         }
 
         public byte[] getBinaryKey() {
-            return this.binaryKeys.get(0);
+            if (binaryKeys != null)
+                return binaryKeys.get(0);
+            else
+                return null;
         }
 
     }