[
https://issues.apache.org/jira/browse/GEODE-8786?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17259731#comment-17259731
]
ASF GitHub Bot commented on GEODE-8786:
---------------------------------------
jdeppe-pivotal commented on a change in pull request #5875:
URL: https://github.com/apache/geode/pull/5875#discussion_r552236162
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
Review comment:
This could also just end up being a call to
`runCommandAndAssertNoStatUpdates`
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
Review comment:
I don't think we need tests for these 3 commands since they don't deal
with keys at all and don't even manipulate any data.
##########
File path:
geode-redis/src/main/java/org/apache/geode/redis/internal/data/CommandHelper.java
##########
@@ -137,37 +140,49 @@ private RedisString checkStringType(RedisData redisData,
boolean ignoreTypeMisma
return (RedisString) redisData;
}
- RedisString getRedisString(ByteArrayWrapper key) {
+ RedisString getRedisString(ByteArrayWrapper key, boolean updateStats) {
RedisData redisData = getRedisData(key, NULL_REDIS_STRING);
- if (redisData == NULL_REDIS_STRING) {
- redisStats.incKeyspaceMisses();
- } else {
- redisStats.incKeyspaceHits();
+ if (updateStats) {
+ if (redisData == NULL_REDIS_STRING) {
+ redisStats.incKeyspaceMisses();
+ } else {
+ redisStats.incKeyspaceHits();
+ }
}
return checkStringType(redisData, false);
}
- RedisString getRedisStringIgnoringType(ByteArrayWrapper key) {
+ RedisString getRedisStringIgnoringType(ByteArrayWrapper key, boolean
updateStats) {
RedisData redisData = getRedisData(key, NULL_REDIS_STRING);
- if (redisData == NULL_REDIS_STRING) {
- redisStats.incKeyspaceMisses();
- } else {
- redisStats.incKeyspaceHits();
+ if (updateStats) {
+ if (redisData == NULL_REDIS_STRING) {
+ redisStats.incKeyspaceMisses();
+ } else {
+ redisStats.incKeyspaceHits();
+ }
}
+
return checkStringType(redisData, true);
}
- RedisString setRedisString(ByteArrayWrapper key, ByteArrayWrapper value) {
+ RedisString setRedisString(ByteArrayWrapper key, ByteArrayWrapper value,
boolean updateStats) {
Review comment:
It looks like this method is only ever called with `updateStats ==
false` so that parameter can be removed. Since it's also a method that is
mutating data it should never be updating the hits/misses stats.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPSubscribeAndPUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis pSubscriber = new Jedis("localhost", getPort(),
REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> pSubscriber.psubscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.punsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPublish() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ Jedis publisher = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ publisher.publish("someChannel", "hello");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+ }
+ }
+
+ // ------------ Key related commands -----------
+
+ @Test
+ public void testExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expire(k, 5));
+ }
+
+ @Test
+ public void testPassiveExpiration() {
+ Map<String, String> info = getInfo(jedis);
+ String currentHits = info.get(HITS);
+ String currentMisses = info.get(MISSES);
+
+ jedis.expire("hash", 1);
+
+ GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(currentHits);
+ assertThat(info.get(MISSES)).isEqualTo(currentMisses);
+ }
+
+ @Test
+ public void testExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expireAt(k,
2145916800));
+ }
+
+ @Test
+ public void testPExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpire(k, 1024));
+ }
+
+ @Test
+ public void testPExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpireAt(k,
1608247597));
+ }
+
+ @Test
+ public void testPersist() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.persist(k));
+ }
+
+ // ------------ Helper Methods -----------
+
private void runCommandAndAssertHitsAndMisses(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ command.accept(key);
+ info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runCommandAndAssertHitsAndMisses(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed", "42");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runDiffCommandAndAssertHitsAndMisses(String key,
BiConsumer<String, String> command) {
- command.accept(key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("2");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 2));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept(key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("3");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 3));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
/**
* When storing diff-ish results, hits and misses are never updated
*/
private void runDiffStoreCommandAndAssertNoStatUpdates(String key,
TriConsumer<String, String, String> command) {
- command.accept("destination", key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept("destination", key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("destination", key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
Review comment:
Since these are just string comparisons there's probably no need to do
any numeric conversions.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -82,6 +81,19 @@ public void testPttl() {
runCommandAndAssertHitsAndMisses("string", k -> jedis.pttl(k));
}
+ @Test
+ public void testRename() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.rename("string", "newString");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
Review comment:
This could probably just become:
```
runCommandAndAssertNoStatUpdates("string", (k, v) -> jedis.rename(k, v));
```
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPSubscribeAndPUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis pSubscriber = new Jedis("localhost", getPort(),
REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> pSubscriber.psubscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.punsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPublish() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ Jedis publisher = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ publisher.publish("someChannel", "hello");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+ }
+ }
+
+ // ------------ Key related commands -----------
+
+ @Test
+ public void testExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expire(k, 5));
+ }
+
+ @Test
+ public void testPassiveExpiration() {
+ Map<String, String> info = getInfo(jedis);
+ String currentHits = info.get(HITS);
+ String currentMisses = info.get(MISSES);
+
+ jedis.expire("hash", 1);
+
+ GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
Review comment:
Can also just use `runCommandAndAssertNoStatUpdates1
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
Review comment:
Ditto for any pub/sub commands - they never deal with keys so I don't
think it makes sense to test them here.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPSubscribeAndPUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis pSubscriber = new Jedis("localhost", getPort(),
REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> pSubscriber.psubscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.punsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPublish() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ Jedis publisher = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ publisher.publish("someChannel", "hello");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+ }
+ }
+
+ // ------------ Key related commands -----------
+
+ @Test
+ public void testExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expire(k, 5));
+ }
+
+ @Test
+ public void testPassiveExpiration() {
+ Map<String, String> info = getInfo(jedis);
+ String currentHits = info.get(HITS);
+ String currentMisses = info.get(MISSES);
+
+ jedis.expire("hash", 1);
+
+ GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(currentHits);
+ assertThat(info.get(MISSES)).isEqualTo(currentMisses);
+ }
+
+ @Test
+ public void testExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expireAt(k,
2145916800));
+ }
+
+ @Test
+ public void testPExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpire(k, 1024));
+ }
+
+ @Test
+ public void testPExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpireAt(k,
1608247597));
+ }
+
+ @Test
+ public void testPersist() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.persist(k));
+ }
+
+ // ------------ Helper Methods -----------
+
private void runCommandAndAssertHitsAndMisses(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ command.accept(key);
+ info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runCommandAndAssertHitsAndMisses(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed", "42");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runDiffCommandAndAssertHitsAndMisses(String key,
BiConsumer<String, String> command) {
- command.accept(key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("2");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 2));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept(key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("3");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 3));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
/**
* When storing diff-ish results, hits and misses are never updated
*/
private void runDiffStoreCommandAndAssertNoStatUpdates(String key,
TriConsumer<String, String, String> command) {
- command.accept("destination", key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept("destination", key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("destination", key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
Review comment:
Since these are just string comparisons there's probably no need to do
any numeric conversions.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPSubscribeAndPUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis pSubscriber = new Jedis("localhost", getPort(),
REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> pSubscriber.psubscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.punsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPublish() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ Jedis publisher = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ publisher.publish("someChannel", "hello");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+ }
+ }
+
+ // ------------ Key related commands -----------
+
+ @Test
+ public void testExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expire(k, 5));
+ }
+
+ @Test
+ public void testPassiveExpiration() {
+ Map<String, String> info = getInfo(jedis);
+ String currentHits = info.get(HITS);
+ String currentMisses = info.get(MISSES);
+
+ jedis.expire("hash", 1);
+
+ GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(currentHits);
+ assertThat(info.get(MISSES)).isEqualTo(currentMisses);
+ }
+
+ @Test
+ public void testExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expireAt(k,
2145916800));
+ }
+
+ @Test
+ public void testPExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpire(k, 1024));
+ }
+
+ @Test
+ public void testPExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpireAt(k,
1608247597));
+ }
+
+ @Test
+ public void testPersist() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.persist(k));
+ }
+
+ // ------------ Helper Methods -----------
+
private void runCommandAndAssertHitsAndMisses(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ command.accept(key);
+ info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runCommandAndAssertHitsAndMisses(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed", "42");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runDiffCommandAndAssertHitsAndMisses(String key,
BiConsumer<String, String> command) {
- command.accept(key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("2");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 2));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept(key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("3");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 3));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
/**
* When storing diff-ish results, hits and misses are never updated
*/
private void runDiffStoreCommandAndAssertNoStatUpdates(String key,
TriConsumer<String, String, String> command) {
- command.accept("destination", key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept("destination", key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("destination", key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
Review comment:
Since these are just string comparisons there's probably no need to do
any numeric conversions.
##########
File path:
geode-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -286,90 +306,312 @@ public void testHscan() {
runCommandAndAssertHitsAndMisses("hash", (k, v) -> jedis.hscan(k, v));
}
+ @Test
+ public void testHMSet() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Map<String, String> map = new HashMap<>();
+ map.put("key1", "value1");
+ map.put("key2", "value2");
+
+ jedis.hmset("key", map);
+
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ Connection related commands -----------
+
+ // TODO auth needs some setup. manually verified that auth does not change
hit/miss stats
+ // if that changes we'll need to come back to this test
+ @Ignore
+ @Test
+ public void testAuth() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.auth("some password");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testPing() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.ping();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ @Test
+ public void testQuit() {
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ jedis.quit();
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+
+ // ------------ PubSub related commands -----------
+
+ // TODO test pubsub related commands
+ // tests are currently flaky when run against Native Redis so they have been
+ // ignored for now. Add them back when GEODE-8577 has been resolved
+ @Ignore
+ @Test
+ public void testSubscribeAndUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPSubscribeAndPUnsubscribe() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis pSubscriber = new Jedis("localhost", getPort(),
REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> pSubscriber.psubscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.punsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ }
+ }
+
+ @Ignore
+ @Test
+ public void testPublish() {
+ MockSubscriber mockSubscriber = new MockSubscriber();
+ Jedis subscriber = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+ Jedis publisher = new Jedis("localhost", getPort(), REDIS_CLIENT_TIMEOUT);
+
+ Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ Runnable runnable = () -> subscriber.subscribe(mockSubscriber,
"someChannel");
+ Thread subscriberThread = new Thread(runnable);
+ subscriberThread.start();
+
+ try {
+ publisher.publish("someChannel", "hello");
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
+ } finally {
+ mockSubscriber.unsubscribe("someChannel");
+ GeodeAwaitility.await().ignoreExceptions().until(() ->
!subscriberThread.isAlive());
+ }
+ }
+
+ // ------------ Key related commands -----------
+
+ @Test
+ public void testExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expire(k, 5));
+ }
+
+ @Test
+ public void testPassiveExpiration() {
+ Map<String, String> info = getInfo(jedis);
+ String currentHits = info.get(HITS);
+ String currentMisses = info.get(MISSES);
+
+ jedis.expire("hash", 1);
+
+ GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
+
+ info = getInfo(jedis);
+ assertThat(info.get(HITS)).isEqualTo(currentHits);
+ assertThat(info.get(MISSES)).isEqualTo(currentMisses);
+ }
+
+ @Test
+ public void testExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.expireAt(k,
2145916800));
+ }
+
+ @Test
+ public void testPExpire() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpire(k, 1024));
+ }
+
+ @Test
+ public void testPExpireAt() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.pexpireAt(k,
1608247597));
+ }
+
+ @Test
+ public void testPersist() {
+ runCommandAndAssertNoStatUpdates("hash", (k) -> jedis.persist(k));
+ }
+
+ // ------------ Helper Methods -----------
+
private void runCommandAndAssertHitsAndMisses(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ command.accept(key);
+ info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runCommandAndAssertHitsAndMisses(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("missed", "42");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("1");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 1));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
private void runDiffCommandAndAssertHitsAndMisses(String key,
BiConsumer<String, String> command) {
- command.accept(key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("2");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 2));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept(key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("3");
- assertThat(info.get(MISSES)).isEqualTo("1");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits + 3));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses + 1));
}
/**
* When storing diff-ish results, hits and misses are never updated
*/
private void runDiffStoreCommandAndAssertNoStatUpdates(String key,
TriConsumer<String, String, String> command) {
- command.accept("destination", key, key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept("destination", key, key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
command.accept("destination", key, "missed");
info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key, Consumer<String>
command) {
- command.accept(key);
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key);
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key, BiConsumer<String,
String> command) {
- command.accept(key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ command.accept(key, "42");
+ info = getInfo(jedis);
+
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
}
private void runCommandAndAssertNoStatUpdates(String key,
TriConsumer<String, String, String> command) {
- command.accept(key, key, "42");
Map<String, String> info = getInfo(jedis);
+ Long currentHits = Long.parseLong(info.get(HITS));
+ Long currentMisses = Long.parseLong(info.get(MISSES));
+
+ command.accept(key, key, "42");
+ info = getInfo(jedis);
- assertThat(info.get(HITS)).isEqualTo("0");
- assertThat(info.get(MISSES)).isEqualTo("0");
+ assertThat(info.get(HITS)).isEqualTo(String.valueOf(currentHits));
+ assertThat(info.get(MISSES)).isEqualTo(String.valueOf(currentMisses));
Review comment:
Since these are just string comparisons there's probably no need to do
any numeric conversions.
----------------------------------------------------------------
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
> INFO command hit/miss ratios should match native Redis
> ------------------------------------------------------
>
> Key: GEODE-8786
> URL: https://issues.apache.org/jira/browse/GEODE-8786
> Project: Geode
> Issue Type: Bug
> Components: redis
> Affects Versions: 1.14.0
> Reporter: Raymond Ingles
> Priority: Major
> Labels: pull-request-available
>
> Current hit/miss ratios for Geode Redis stats do not match those for native
> Redis.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
