[ 
https://issues.apache.org/jira/browse/GEODE-8663?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17223151#comment-17223151
 ] 

ASF GitHub Bot commented on GEODE-8663:
---------------------------------------

jhutchison commented on a change in pull request #5678:
URL: https://github.com/apache/geode/pull/5678#discussion_r514498015



##########
File path: 
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/RedisStatsIntegrationTest.java
##########
@@ -16,28 +16,469 @@
 
 import static org.assertj.core.api.Assertions.assertThat;
 
+import java.util.concurrent.TimeUnit;
+
+import org.junit.Before;
 import org.junit.ClassRule;
 import org.junit.Test;
+import redis.clients.jedis.BitOP;
 import redis.clients.jedis.Jedis;
 
 import org.apache.geode.redis.GeodeRedisServerRule;
 import org.apache.geode.test.awaitility.GeodeAwaitility;
 
 public class RedisStatsIntegrationTest {
+  public static final String EXISTING_HASH_KEY = "Existing_Hash";
+  public static final String EXISTING_STRING_KEY = "Existing_String";
+  public static final String EXISTING_SET_KEY_1 = "Existing_Set_1";
+  public static final String EXISTING_SET_KEY_2 = "Existing_Set_2";
+  public static final String NONEXISTENT_KEY = "Nonexistent_Key";
+  Jedis jedis;
+  long initialKeyspaceHits;
+  long initialKeyspaceMisses;
 
   @ClassRule
   public static GeodeRedisServerRule server = new GeodeRedisServerRule();
 
+  @Before
+  public void setup() {
+    jedis = new Jedis("localhost", server.getPort(), 10000000);
+    jedis.flushAll();
+    jedis.set(EXISTING_STRING_KEY, "A_Value");
+    jedis.hset(EXISTING_HASH_KEY, "Field1", "Value1");
+    jedis.sadd(EXISTING_SET_KEY_1, "m1", "m2", "m3");
+    jedis.sadd(EXISTING_SET_KEY_2, "m4", "m5", "m6");
+    initialKeyspaceHits = server.getServer().getStats().getKeyspaceHits();
+    initialKeyspaceMisses = server.getServer().getStats().getKeyspaceMisses();
+  }
+
   @Test
-  public void clientsStat_withConnectAndClose_isCorrect() throws 
InterruptedException {
+  public void clientsStat_withConnectAndClose_isCorrect() {
     long initialClients = server.getServer().getStats().getClients();
     Jedis jedis = new Jedis("localhost", server.getPort(), 10000000);
 
     jedis.ping();
     
assertThat(server.getServer().getStats().getClients()).isEqualTo(initialClients 
+ 1);
 
     jedis.close();
-    GeodeAwaitility.await().untilAsserted(
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
         () -> 
assertThat(server.getServer().getStats().getClients()).isEqualTo(initialClients));
   }
+
+  @Test
+  public void keyspaceHitsStat_shouldIncrement_whenKeyAccessed() {
+    jedis.get(EXISTING_STRING_KEY);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceHitsStat_shouldNotIncrement_whenNonexistentKeyAccessed() 
{
+    jedis.get("Nonexistent_Key");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  // TODO: Set doesn't work like native Redis!
+  @Test
+  public void keyspaceStats_setCommand_existingKey() {
+    jedis.set(EXISTING_STRING_KEY, "New_Value");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  // TODO: Set doesn't work like native Redis!
+  @Test
+  public void keyspaceStats_setCommand_nonexistentKey() {
+    jedis.set("Another_Key", "Another_Value");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getBitCommand_existingKey() {
+    jedis.getbit(EXISTING_STRING_KEY, 0);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getBitCommand_nonexistentKey() {
+    jedis.getbit("Nonexistent_Key", 0);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getRangeCommand_existingKey() {
+    jedis.getrange(EXISTING_STRING_KEY, 0, 1);
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getRangeCommand_nonexistentKey() {
+    jedis.getrange("Nonexistent_Key", 0, 1);
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getSetCommand_existingKey() {
+    jedis.getSet(EXISTING_STRING_KEY, "New_Value");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_getSetCommand_nonexistentKey() {
+    jedis.getSet("Nonexistent_Key", "FakeValue");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_strlenCommand_existingKey() {
+    jedis.strlen(EXISTING_STRING_KEY);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_strlenCommand_nonexistentKey() {
+    jedis.strlen(NONEXISTENT_KEY);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_mgetCommand() {
+    jedis.mget(EXISTING_STRING_KEY, "Nonexistent_Key");
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_bitopCommand() {
+    jedis.bitop(BitOP.AND, EXISTING_STRING_KEY, EXISTING_STRING_KEY, 
"Nonexistent_Key");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 2);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_bitcountCommand_existingKey() {
+    jedis.bitcount(EXISTING_STRING_KEY);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_bitcountCommand_nonexistentKey() {
+    jedis.bitcount("Nonexistent_Key");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_bitposCommand_existingKey() {
+    jedis.bitpos(EXISTING_STRING_KEY, true);
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_bitposCommand_nonexistentKey() {
+    jedis.bitpos("Nonexistent_Key", true);
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_hgetCommand_existingKey() {
+    jedis.hget(EXISTING_HASH_KEY, "Field1");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_hgetCommand_nonexistentKey() {
+    jedis.hget("Nonexistent_Hash", "Field1");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_smembersCommand_existingKey() {
+    jedis.smembers(EXISTING_SET_KEY_1);
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 1);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_smembersCommand_nonexistentKey() {
+    jedis.smembers("Nonexistent_Set");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);
+        });
+  }
+
+  @Test
+  public void keyspaceStats_sunionstoreCommand_existingKey() {
+    jedis.sunionstore("New_Set", EXISTING_SET_KEY_1, EXISTING_SET_KEY_2, 
"Nonexistent_Set");
+
+    jedis.close();
+    GeodeAwaitility.await().atMost(5, TimeUnit.SECONDS).untilAsserted(
+        () -> {
+          assertThat(server.getServer().getStats().getKeyspaceHits())
+              .isEqualTo(initialKeyspaceHits + 2);
+          assertThat(server.getServer().getStats().getKeyspaceMisses())
+              .isEqualTo(initialKeyspaceMisses + 1);

Review comment:
       > why does this not cause two misses?
   this is a weird interface- the first parameter is the set that the union 
will be stored in - so no hit or miss- just created. the 2 hits come from the 
existing keys. the miss comes form the non-existent set. let us know if that 
still seems weird and isn't addressed in a new story




----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


> update Redis Info command To include additional statistics
> ----------------------------------------------------------
>
>                 Key: GEODE-8663
>                 URL: https://issues.apache.org/jira/browse/GEODE-8663
>             Project: Geode
>          Issue Type: Improvement
>          Components: redis
>            Reporter: John Hutchison
>            Priority: Major
>              Labels: pull-request-available
>




--
This message was sent by Atlassian Jira
(v8.3.4#803005)

Reply via email to