From 714f765385b86de34b2cb4fd3740a8231de2e97c Mon Sep 17 00:00:00 2001
From: Oleksii Moskalenko <moskalenko.alexey@gmail.com>
Date: Wed, 16 Sep 2020 12:21:26 +0800
Subject: [PATCH] call fallback only when theres missing keys

---
 .../redis/retriever/RedisClusterOnlineRetriever.java     | 4 ++++
 .../redis/retriever/RedisClusterOnlineRetrieverTest.java | 9 +++++++--
 2 files changed, 11 insertions(+), 2 deletions(-)

diff --git a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetriever.java b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetriever.java
index 8170adc9d2b..c49745bbbd1 100644
--- a/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetriever.java
+++ b/storage/connectors/redis/src/main/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetriever.java
@@ -228,6 +228,10 @@ private List<byte[]> sendMultiGet(List<RedisKey> keys) {
                 .boxed()
                 .collect(Collectors.toList());
 
+        if (indexMissingValue.isEmpty()) {
+          return redisValues;
+        }
+
         byte[][] fallbackBinaryKeys =
             indexMissingValue.stream()
                 .map(i -> fallbackSerializer.serialize(keys.get(i)))
diff --git a/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetrieverTest.java b/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetrieverTest.java
index fc042cd9240..419ce8e0a95 100644
--- a/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetrieverTest.java
+++ b/storage/connectors/redis/src/test/java/feast/storage/connectors/redis/retriever/RedisClusterOnlineRetrieverTest.java
@@ -18,7 +18,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.equalTo;
-import static org.mockito.Mockito.when;
+import static org.mockito.Mockito.*;
 import static org.mockito.MockitoAnnotations.initMocks;
 
 import com.google.common.collect.ImmutableList;
@@ -147,7 +147,9 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
     List<KeyValue<byte[], byte[]>> featureRowBytes = Lists.newArrayList(keyValue1, keyValue2);
 
     OnlineRetriever redisClusterOnlineRetriever =
-        new RedisClusterOnlineRetriever.Builder(connection, serializer).build();
+        new RedisClusterOnlineRetriever.Builder(connection, serializer)
+            .withFallbackSerializer(fallbackSerializer)
+            .build();
     when(syncCommands.mget(serializedKey1, serializedKey2)).thenReturn(featureRowBytes);
 
     List<Optional<FeatureRow>> expected =
@@ -174,6 +176,9 @@ public void shouldReturnResponseWithValuesIfKeysPresent() {
     List<Optional<FeatureRow>> actual =
         redisClusterOnlineRetriever.getOnlineFeatures(entityRows, featureSetRequest);
     assertThat(actual, equalTo(expected));
+
+    // check that fallback is used only when there's something to fallback
+    verify(syncCommands, never()).mget();
   }
 
   @Test