Skip to content

Fix memory leak in JedisClusterInfoCache - replica nodes not cleared #4205

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jul 20, 2025
Merged

Fix memory leak in JedisClusterInfoCache - replica nodes not cleared #4205

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
034c910
Update reset on JedisClusterInfoCache
thachlp Jul 19, 2025
31b9ce5
Fix the test
thachlp Jul 19, 2025
b978e8e
Add unit test for JedisClusterInfoCache
ggivo Jul 19, 2025
4c0cb13
revert formating only changes
ggivo Jul 20, 2025
a1f5fb6
format updated code
ggivo Jul 20, 2025
a8ef73b
Merge branch 'master' into update-reset-method
ggivo Jul 20, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
30 changes: 22 additions & 8 deletions src/main/java/redis/clients/jedis/JedisClusterInfoCache.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,8 @@ public void run() {
}
}

public JedisClusterInfoCache(final JedisClientConfig clientConfig, final Set<HostAndPort> startNodes) {
public JedisClusterInfoCache(final JedisClientConfig clientConfig,
final Set<HostAndPort> startNodes) {
this(clientConfig, null, null, startNodes);
}

Expand Down Expand Up @@ -107,10 +108,11 @@ public JedisClusterInfoCache(final JedisClientConfig clientConfig, Cache clientS
clientConfig.getAuthXManager().start();
}
if (topologyRefreshPeriod != null) {
logger.info("Cluster topology refresh start, period: {}, startNodes: {}", topologyRefreshPeriod, startNodes);
logger.info("Cluster topology refresh start, period: {}, startNodes: {}",
topologyRefreshPeriod, startNodes);
topologyRefreshExecutor = Executors.newSingleThreadScheduledExecutor();
topologyRefreshExecutor.scheduleWithFixedDelay(new TopologyRefreshTask(), topologyRefreshPeriod.toMillis(),
topologyRefreshPeriod.toMillis(), TimeUnit.MILLISECONDS);
topologyRefreshExecutor.scheduleWithFixedDelay(new TopologyRefreshTask(),
topologyRefreshPeriod.toMillis(), topologyRefreshPeriod.toMillis(), TimeUnit.MILLISECONDS);
}
if (clientConfig.isReadOnlyForRedisClusterReplicas()) {
replicaSlots = new ArrayList[Protocol.CLUSTER_HASHSLOTS];
Expand All @@ -120,14 +122,15 @@ public JedisClusterInfoCache(final JedisClientConfig clientConfig, Cache clientS
}

/**
* Check whether the number and order of slots in the cluster topology are equal to CLUSTER_HASHSLOTS
* Check whether the number and order of slots in the cluster topology are equal to
* CLUSTER_HASHSLOTS
* @param slotsInfo the cluster topology
* @return if slots is ok, return true, elese return false.
*/
private boolean checkClusterSlotSequence(List<Object> slotsInfo) {
List<Integer> slots = new ArrayList<>();
for (Object slotInfoObj : slotsInfo) {
List<Object> slotInfo = (List<Object>)slotInfoObj;
List<Object> slotInfo = (List<Object>) slotInfoObj;
slots.addAll(getAssignedSlotArray(slotInfo));
}
Collections.sort(slots);
Expand Down Expand Up @@ -187,7 +190,8 @@ public void discoverClusterNodesAndSlots(Connection jedis) {
}

public void renewClusterSlots(Connection jedis) {
// If rediscovering is already in process - no need to start one more same rediscovering, just return
// If rediscovering is already in process - no need to start one more same rediscovering, just
// return
if (rediscoverLock.tryLock()) {
try {
// First, if jedis is available, use jedis renew.
Expand Down Expand Up @@ -454,6 +458,16 @@ public void reset() {
nodes.clear();
Arrays.fill(slots, null);
Arrays.fill(slotNodes, null);
if (clientSideCache != null) {
clientSideCache.flush();
}
if (clientConfig.isReadOnlyForRedisClusterReplicas() && replicaSlots != null) {
for (List<ConnectionPool> replicaList : replicaSlots) {
if (replicaList != null) {
replicaList.clear();
}
}
}
} finally {
w.unlock();
}
Expand All @@ -468,7 +482,7 @@ public void close() {
}

public static String getNodeKey(HostAndPort hnp) {
//return hnp.getHost() + ":" + hnp.getPort();
// return hnp.getHost() + ":" + hnp.getPort();
return hnp.toString();
}

Expand Down
62 changes: 35 additions & 27 deletions src/test/java/redis/clients/jedis/commands/jedis/ControlCommandsTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThan;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertInstanceOf;
Expand Down Expand Up @@ -148,8 +149,8 @@ public void roleMaster() {
@Test
public void roleSlave() {
EndpointConfig primaryEndpoint = HostAndPorts.getRedisEndpoint("standalone0");
EndpointConfig secondaryEndpoint = HostAndPorts.getRedisEndpoint(
"standalone4-replica-of-standalone1");
EndpointConfig secondaryEndpoint = HostAndPorts
.getRedisEndpoint("standalone4-replica-of-standalone1");

try (Jedis slave = new Jedis(secondaryEndpoint.getHostAndPort(),
secondaryEndpoint.getClientConfigBuilder().build())) {
Expand Down Expand Up @@ -226,18 +227,18 @@ public void configGet() {
Map<String, String> info = jedis.configGet("m*");
assertNotNull(info);
assertFalse(info.isEmpty());
// assertTrue(info.size() % 2 == 0);
// assertTrue(info.size() % 2 == 0);
Map<byte[], byte[]> infoBinary = jedis.configGet("m*".getBytes());
assertNotNull(infoBinary);
assertFalse(infoBinary.isEmpty());
// assertTrue(infoBinary.size() % 2 == 0);
// assertTrue(infoBinary.size() % 2 == 0);
}

@Test
public void configSet() {
Map<String, String> info = jedis.configGet("maxmemory");
// assertEquals("maxmemory", info.get(0));
// String memory = info.get(1);
// assertEquals("maxmemory", info.get(0));
// String memory = info.get(1);
String memory = info.get("maxmemory");
assertNotNull(memory);
assertEquals("OK", jedis.configSet("maxmemory", "200"));
Expand All @@ -248,8 +249,8 @@ public void configSet() {
public void configSetBinary() {
byte[] maxmemory = SafeEncoder.encode("maxmemory");
Map<byte[], byte[]> info = jedis.configGet(maxmemory);
// assertArrayEquals(maxmemory, info.get(0));
// byte[] memory = info.get(1);
// assertArrayEquals(maxmemory, info.get(0));
// byte[] memory = info.get(1);
byte[] memory = info.get(maxmemory);
assertNotNull(memory);
assertEquals("OK", jedis.configSet(maxmemory, Protocol.toByteArray(200)));
Expand All @@ -259,13 +260,15 @@ public void configSetBinary() {
@Test
@SinceRedisVersion(value = "7.0.0", message = "Starting with Redis version 7.0.0: Added the ability to pass multiple pattern parameters in one call")
public void configGetSetMulti() {
String[] params = new String[]{"hash-max-listpack-entries", "set-max-intset-entries", "zset-max-listpack-entries"};
String[] params = new String[] { "hash-max-listpack-entries", "set-max-intset-entries",
"zset-max-listpack-entries" };
Map<String, String> info = jedis.configGet(params);
assertEquals(3, info.size());
assertEquals("OK", jedis.configSet(info));

byte[][] bparams = new byte[][]{SafeEncoder.encode("hash-max-listpack-entries"),
SafeEncoder.encode("set-max-intset-entries"), SafeEncoder.encode("zset-max-listpack-entries")};
byte[][] bparams = new byte[][] { SafeEncoder.encode("hash-max-listpack-entries"),
SafeEncoder.encode("set-max-intset-entries"),
SafeEncoder.encode("zset-max-listpack-entries") };
Map<byte[], byte[]> binfo = jedis.configGet(bparams);
assertEquals(3, binfo.size());
assertEquals("OK", jedis.configSetBinary(binfo));
Expand Down Expand Up @@ -429,23 +432,23 @@ public void memoryUsageBinary() {
// Note: It has been recommended not to base MEMORY USAGE test on exact value, as the response
// may subject to be 'tuned' especially targeting a major Redis release.

byte[] bfoo = {0x01, 0x02, 0x03, 0x04};
byte[] bbar = {0x05, 0x06, 0x07, 0x08};
byte[] bfoobar = {0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08};
byte[] bfoo = { 0x01, 0x02, 0x03, 0x04 };
byte[] bbar = { 0x05, 0x06, 0x07, 0x08 };
byte[] bfoobar = { 0x01, 0x02, 0x03, 0x04, 0x05, 0x06, 0x07, 0x08 };

jedis.set(bfoo, bbar);
assertThat(jedis.memoryUsage(bfoo), greaterThan(20l));

jedis.lpush(bfoobar, new byte[]{0x01, 0x02}, new byte[]{0x05, 0x06}, new byte[]{0x00});
assertThat(jedis.memoryUsage(bfoobar, 2), greaterThan(40l));
jedis.lpush(bfoobar, new byte[] { 0x01, 0x02 }, new byte[] { 0x05, 0x06 }, new byte[] { 0x00 });
assertThat(jedis.memoryUsage(bfoobar, 2), greaterThanOrEqualTo(40l));

assertNull(jedis.memoryUsage("roo", 2));
}

@Test
public void memoryPurge() {
String memoryPurge = jedis.memoryPurge();
assertNotNull(memoryPurge);
String memoryPurge = jedis.memoryPurge();
assertNotNull(memoryPurge);
}

@Test
Expand Down Expand Up @@ -489,15 +492,16 @@ public void commandDocs() {

CommandDocument sortDoc = docs.get("sort");
assertEquals("generic", sortDoc.getGroup());
MatcherAssert.assertThat(sortDoc.getSummary(), Matchers.isOneOf(
"Sort the elements in a list, set or sorted set",
MatcherAssert.assertThat(sortDoc.getSummary(),
Matchers.isOneOf("Sort the elements in a list, set or sorted set",
"Sorts the elements in a list, a set, or a sorted set, optionally storing the result."));
assertNull(sortDoc.getHistory());

CommandDocument setDoc = docs.get("set");
assertEquals("1.0.0", setDoc.getSince());
assertEquals("O(1)", setDoc.getComplexity());
assertEquals("2.6.12: Added the `EX`, `PX`, `NX` and `XX` options.", setDoc.getHistory().get(0));
assertEquals("2.6.12: Added the `EX`, `PX`, `NX` and `XX` options.",
setDoc.getHistory().get(0));
}

@Test
Expand All @@ -506,7 +510,8 @@ public void commandGetKeys() {
List<String> keys = jedis.commandGetKeys("SORT", "mylist", "ALPHA", "STORE", "outlist");
assertEquals(2, keys.size());

List<KeyValue<String, List<String>>> keySandFlags = jedis.commandGetKeysAndFlags("SET", "k1", "v1");
List<KeyValue<String, List<String>>> keySandFlags = jedis.commandGetKeysAndFlags("SET", "k1",
"v1");
assertEquals("k1", keySandFlags.get(0).getKey());
assertEquals(2, keySandFlags.get(0).getValue().size());
}
Expand Down Expand Up @@ -581,17 +586,20 @@ public void commandList() {
List<String> commands = jedis.commandList();
assertTrue(commands.size() > 100);

commands = jedis.commandListFilterBy(CommandListFilterByParams.commandListFilterByParams().filterByModule("JSON"));
commands = jedis.commandListFilterBy(
CommandListFilterByParams.commandListFilterByParams().filterByModule("JSON"));
assertEquals(0, commands.size()); // json module was not loaded

commands = jedis.commandListFilterBy(CommandListFilterByParams.commandListFilterByParams().filterByAclCat("admin"));
commands = jedis.commandListFilterBy(
CommandListFilterByParams.commandListFilterByParams().filterByAclCat("admin"));
assertTrue(commands.size() > 10);

commands = jedis.commandListFilterBy(CommandListFilterByParams.commandListFilterByParams().filterByPattern("a*"));
commands = jedis.commandListFilterBy(
CommandListFilterByParams.commandListFilterByParams().filterByPattern("a*"));
assertTrue(commands.size() > 10);

assertThrows(IllegalArgumentException.class, () ->
jedis.commandListFilterBy(CommandListFilterByParams.commandListFilterByParams()));
assertThrows(IllegalArgumentException.class,
() -> jedis.commandListFilterBy(CommandListFilterByParams.commandListFilterByParams()));
}

@Test
Expand Down
Loading