sabbey37 commented on a change in pull request #6325:
URL: https://github.com/apache/geode/pull/6325#discussion_r618624714



##########
File path: 
geode-apis-compatible-with-redis/src/integrationTest/java/org/apache/geode/redis/internal/executor/server/AbstractHitsMissesIntegrationTest.java
##########
@@ -168,7 +169,7 @@ public void testExpire() {
   @Test
   public void testPassiveExpiration() {
     runCommandAndAssertNoStatUpdates("hash", (k) -> {
-      jedis.expire(k, 1);
+      jedis.expire(k, PassiveExpirationManager.INTERVAL);
       GeodeAwaitility.await().during(Duration.ofSeconds(3)).until(() -> true);
     });
   }

Review comment:
       It seems like the intention of this test is to ensure keyspace hits and 
misses don't update after the `EXPIRE` command and after the key passively 
expires. We can leave the time we set the key to expire as 1 second 
(`jedis.expire(k, 1);`).  However, we need to increase the time we are waiting 
for the key to expire passively since that passive expiration interval has 
changed.
   
   Perhaps we could change the await to be something like this:
   
   ```
    GeodeAwaitility.await()
             .atMost(Duration.ofMinutes(PassiveExpirationManager.INTERVAL))
             .until(() -> jedis.keys("hash").isEmpty());
   ```
   ...so we're verifying the key expires before checking the keyspace hits and 
misses. We will need to add some time to that INTERVAL in the await for a 
buffer and to avoid potential flakiness.




-- 
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:
[email protected]


Reply via email to