Skip to content

Commit

Permalink
GEODE-10047: Fix expiration interval log (#7359)
Browse files Browse the repository at this point in the history
- 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 <[email protected]>
(cherry picked from commit cd0b52e)
  • Loading branch information
DonalEvans committed Feb 15, 2022
1 parent 5a656ce commit 9f4f66e
Show file tree
Hide file tree
Showing 5 changed files with 37 additions and 37 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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;
Expand All @@ -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());
Expand Down Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,28 +34,29 @@
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() {
expirationExecutor.shutdownNow();
}

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();
Expand All @@ -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);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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");
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand All @@ -161,7 +161,6 @@ public void endExpiration(long start) {
expirations.incrementAndGet();
}


public long startPublish() {
return geodeRedisStats.startPublish();
}
Expand Down

0 comments on commit 9f4f66e

Please sign in to comment.