From 9f4f66ed9ed0f8fdba432c6df92184ee5adcc7eb Mon Sep 17 00:00:00 2001 From: Donal Evans Date: Tue, 15 Feb 2022 15:34:27 -0800 Subject: [PATCH] GEODE-10047: Fix expiration interval log (#7359) - Change naming of passive expiration to active expiration throughout the geode-for-redis module so as to be consistent with Redis' language - Replace hard-coded expiration interval of 1 second with a reference to the configured expiration interval in log message in PassiveExpirationManager (now named ActiveExpirationManager) Authored-by: Donal Evans (cherry picked from commit cd0b52e02bb4aeef7b35a2c5015cb7333481232e) --- .../key/AbstractPexpireIntegrationTest.java | 2 +- .../redis/internal/GeodeRedisServer.java | 8 ++--- ...ager.java => ActiveExpirationManager.java} | 21 +++++------ .../internal/statistics/GeodeRedisStats.java | 36 +++++++++---------- .../redis/internal/statistics/RedisStats.java | 7 ++-- 5 files changed, 37 insertions(+), 37 deletions(-) rename geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/{PassiveExpirationManager.java => ActiveExpirationManager.java} (83%) diff --git a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractPexpireIntegrationTest.java b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractPexpireIntegrationTest.java index 3e06c07ef714..d673a73df8c9 100644 --- a/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractPexpireIntegrationTest.java +++ b/geode-for-redis/src/integrationTest/java/org/apache/geode/redis/internal/commands/executor/key/AbstractPexpireIntegrationTest.java @@ -99,7 +99,7 @@ public void should_removeSetKey_AfterExpirationPeriod() { } @Test - public void should_passivelyExpireKeys() { + public void should_activelyExpireKeys() { jedis.sadd("{tag1}key", "value"); jedis.pexpire("{tag1}key", 100); diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java index e53d44fd6919..15bc0cedcf13 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/GeodeRedisServer.java @@ -34,7 +34,7 @@ import org.apache.geode.redis.internal.pubsub.PubSub; import org.apache.geode.redis.internal.pubsub.PubSubImpl; import org.apache.geode.redis.internal.pubsub.Subscriptions; -import org.apache.geode.redis.internal.services.PassiveExpirationManager; +import org.apache.geode.redis.internal.services.ActiveExpirationManager; import org.apache.geode.redis.internal.services.RegionProvider; import org.apache.geode.redis.internal.services.cluster.RedisMemberInfo; import org.apache.geode.redis.internal.services.cluster.RedisMemberInfoRetrievalFunction; @@ -59,7 +59,7 @@ public class GeodeRedisServer { public static final String ENABLE_UNSUPPORTED_COMMANDS_PARAM = "enable-unsupported-commands"; private static boolean unsupportedCommandsEnabled; private static final Logger logger = LogService.getLogger(); - private final PassiveExpirationManager passiveExpirationManager; + private final ActiveExpirationManager activeExpirationManager; private final NettyRedisServer nettyRedisServer; private final RegionProvider regionProvider; private final PubSub pubSub; @@ -86,7 +86,7 @@ public GeodeRedisServer(String bindAddress, int port, InternalCache cache) { regionProvider = new RegionProvider(cache, stripedCoordinator, redisStats); pubSub = new PubSubImpl(new Subscriptions(redisStats), regionProvider, redisStats); - passiveExpirationManager = new PassiveExpirationManager(regionProvider); + activeExpirationManager = new ActiveExpirationManager(regionProvider); DistributedMember member = cache.getDistributedSystem().getDistributedMember(); RedisSecurityService securityService = new RedisSecurityService(cache.getSecurityService()); @@ -159,7 +159,7 @@ public synchronized void shutdown() { if (!shutdown) { logger.info("GeodeRedisServer shutting down"); pubSub.close(); - passiveExpirationManager.stop(); + activeExpirationManager.stop(); nettyRedisServer.stop(); redisStats.close(); shutdown = true; diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/PassiveExpirationManager.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/ActiveExpirationManager.java similarity index 83% rename from geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/PassiveExpirationManager.java rename to geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/ActiveExpirationManager.java index 1bc02b8db952..98bf0f06a1b5 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/PassiveExpirationManager.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/services/ActiveExpirationManager.java @@ -34,20 +34,21 @@ import org.apache.geode.redis.internal.data.RedisData; import org.apache.geode.redis.internal.data.RedisKey; -public class PassiveExpirationManager { +public class ActiveExpirationManager { private static final Logger logger = LogService.getLogger(); public static final int DEFAULT_REDIS_EXPIRATION_INTERVAL_SECONDS = 180; private final ScheduledExecutorService expirationExecutor; + private final int activeExpirationInterval; - public PassiveExpirationManager(RegionProvider regionProvider) { - int interval = getIntegerSystemProperty(EXPIRATION_INTERVAL_SECONDS, + public ActiveExpirationManager(RegionProvider regionProvider) { + activeExpirationInterval = getIntegerSystemProperty(EXPIRATION_INTERVAL_SECONDS, DEFAULT_REDIS_EXPIRATION_INTERVAL_SECONDS, 1); - expirationExecutor = newSingleThreadScheduledExecutor("GemFireRedis-PassiveExpiration-"); - expirationExecutor.scheduleWithFixedDelay(() -> doDataExpiration(regionProvider), interval, - interval, SECONDS); + expirationExecutor = newSingleThreadScheduledExecutor("GemFireRedis-ActiveExpiration-"); + expirationExecutor.scheduleWithFixedDelay(() -> doDataExpiration(regionProvider), + activeExpirationInterval, activeExpirationInterval, SECONDS); } public void stop() { @@ -55,7 +56,7 @@ public void stop() { } private void doDataExpiration(RegionProvider regionProvider) { - final long start = regionProvider.getRedisStats().startPassiveExpirationCheck(); + final long start = regionProvider.getRedisStats().startActiveExpirationCheck(); long expireCount = 0; try { final long now = System.currentTimeMillis(); @@ -76,10 +77,10 @@ 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("Active expiration failed. Will try again in " + activeExpirationInterval + + " second(s).", ex); } finally { - regionProvider.getRedisStats().endPassiveExpirationCheck(start, expireCount); + regionProvider.getRedisStats().endActiveExpirationCheck(start, expireCount); } } diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java index 003cadd89b38..110c969fcf30 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/GeodeRedisStats.java @@ -40,9 +40,9 @@ public class GeodeRedisStats { new EnumMap<>(RedisCommandType.class); private static final int currentlyConnectedClients; - private static final int passiveExpirationChecksId; - private static final int passiveExpirationCheckTimeId; - private static final int passiveExpirationsId; + private static final int activeExpirationChecksId; + private static final int activeExpirationCheckTimeId; + private static final int activeExpirationsId; private static final int expirationsId; private static final int expirationTimeId; private static final int totalConnectionsReceived; @@ -84,9 +84,9 @@ public GeodeRedisStats(StatisticsFactory factory, String name, StatisticsClock c fillTimeIdMap(); currentlyConnectedClients = type.nameToId("connectedClients"); - passiveExpirationChecksId = type.nameToId("passiveExpirationChecks"); - passiveExpirationCheckTimeId = type.nameToId("passiveExpirationCheckTime"); - passiveExpirationsId = type.nameToId("passiveExpirations"); + activeExpirationChecksId = type.nameToId("activeExpirationChecks"); + activeExpirationCheckTimeId = type.nameToId("activeExpirationCheckTime"); + activeExpirationsId = type.nameToId("activeExpirations"); expirationsId = type.nameToId("expirations"); expirationTimeId = type.nameToId("expirationTime"); totalConnectionsReceived = type.nameToId("totalConnectionsReceived"); @@ -122,18 +122,18 @@ public void removeClient() { stats.incLong(currentlyConnectedClients, -1); } - public void endPassiveExpirationCheck(long start, long expireCount) { + public void endActiveExpirationCheck(long start, long expireCount) { if (expireCount > 0) { - incPassiveExpirations(expireCount); + incActiveExpirations(expireCount); } if (clock.isEnabled()) { - stats.incLong(passiveExpirationCheckTimeId, getCurrentTimeNanos() - start); + stats.incLong(activeExpirationCheckTimeId, getCurrentTimeNanos() - start); } - stats.incLong(passiveExpirationChecksId, 1); + stats.incLong(activeExpirationChecksId, 1); } - private void incPassiveExpirations(long count) { - stats.incLong(passiveExpirationsId, count); + private void incActiveExpirations(long count) { + stats.incLong(activeExpirationsId, count); } public void endExpiration(long start) { @@ -255,18 +255,18 @@ private static void fillListWithCommandDescriptors(StatisticsTypeFactory statist "Total number of client connections received by this redis server since startup.", "connections")); - descriptorList.add(statisticsTypeFactory.createLongCounter("passiveExpirationChecks", - "Total number of passive expiration checks that have" + descriptorList.add(statisticsTypeFactory.createLongCounter("activeExpirationChecks", + "Total number of active expiration checks that have" + " completed. Checks include scanning and expiring.", "checks")); - descriptorList.add(statisticsTypeFactory.createLongCounter("passiveExpirationCheckTime", - "Total amount of time, in nanoseconds, spent in passive " + descriptorList.add(statisticsTypeFactory.createLongCounter("activeExpirationCheckTime", + "Total amount of time, in nanoseconds, spent in active " + "expiration checks on this server.", "nanoseconds")); - descriptorList.add(statisticsTypeFactory.createLongCounter("passiveExpirations", - "Total number of keys that have been passively expired on this server.", + descriptorList.add(statisticsTypeFactory.createLongCounter("activeExpirations", + "Total number of keys that have been actively expired on this server.", "expirations")); descriptorList.add(statisticsTypeFactory.createLongCounter("expirations", diff --git a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java index b4317d84f130..35d955739ad9 100644 --- a/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java +++ b/geode-for-redis/src/main/java/org/apache/geode/redis/internal/statistics/RedisStats.java @@ -144,12 +144,12 @@ public long getKeyspaceMisses() { return keyspaceMisses.get(); } - public long startPassiveExpirationCheck() { + public long startActiveExpirationCheck() { return getCurrentTimeNanos(); } - public void endPassiveExpirationCheck(long start, long expireCount) { - geodeRedisStats.endPassiveExpirationCheck(start, expireCount); + public void endActiveExpirationCheck(long start, long expireCount) { + geodeRedisStats.endActiveExpirationCheck(start, expireCount); } public long startExpiration() { @@ -161,7 +161,6 @@ public void endExpiration(long start) { expirations.incrementAndGet(); } - public long startPublish() { return geodeRedisStats.startPublish(); }