dschneider-pivotal commented on a change in pull request #7359:
URL: https://github.com/apache/geode/pull/7359#discussion_r805079523
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java
##########
@@ -144,24 +144,15 @@ public long getKeyspaceMisses() {
return keyspaceMisses.get();
}
- public long startPassiveExpirationCheck() {
- return getCurrentTimeNanos();
- }
-
public void endPassiveExpirationCheck(long start, long expireCount) {
geodeRedisStats.endPassiveExpirationCheck(start, expireCount);
}
- public long startExpiration() {
Review comment:
I would be in favor of keeping these "bookend" start methods. I know
that is this particular case all that start does is return getCurrentTimeNanos.
But I think it is worth preserving the start/end pattern. And it would make it
simple in the future to add other starts that are changed on start (like an in
progress stat). I know these particular start methods can be unwrapped but then
the abstraction is different and it is not as obvious in the caller how the
start and end calls correspond.
##########
File path:
geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/PassiveExpirationManager.java
##########
@@ -76,8 +77,8 @@ private void doDataExpiration(RegionProvider regionProvider) {
}
} catch (CacheClosedException ignore) {
} catch (RuntimeException | Error ex) {
- logger.warn("Passive expiration failed. Will try again in 1 second.",
- ex);
+ logger.warn("Passive expiration failed. Will try again in " +
passiveExpirationInterval
Review comment:
Something you don't need to clean up in this pr but that you remind me
of is that I got the naming of this expiration manager wrong. Native Redis
calls this "active" expiration. The expiration we do when we do an op and find
the key expired is what native redis calls "passive" expiration. So these stats
and log messages may be a bit confusing to native redis users.
--
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.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
