diff --git a/src/main/java/redis/clients/jedis/providers/SentineledConnectionProvider.java b/src/main/java/redis/clients/jedis/providers/SentineledConnectionProvider.java index dedf34fb69..811fa4ed66 100644 --- a/src/main/java/redis/clients/jedis/providers/SentineledConnectionProvider.java +++ b/src/main/java/redis/clients/jedis/providers/SentineledConnectionProvider.java @@ -2,7 +2,9 @@ import java.util.ArrayList; import java.util.Collection; +import java.util.Collections; import java.util.List; +import java.util.Map; import java.util.Set; import java.util.concurrent.atomic.AtomicBoolean; import java.util.concurrent.locks.Lock; @@ -24,6 +26,7 @@ import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisException; import redis.clients.jedis.util.IOUtils; +import redis.clients.jedis.util.Pool; public class SentineledConnectionProvider implements ConnectionProvider { @@ -112,6 +115,16 @@ public Connection getConnection(CommandArguments args) { return pool.getResource(); } + @Override + public Map> getConnectionMap() { + return Collections.singletonMap(currentMaster, pool); + } + + @Override + public Map> getPrimaryNodesConnectionMap() { + return Collections.singletonMap(currentMaster, pool); + } + @Override public void close() { sentinelListeners.forEach(SentinelListener::shutdown); diff --git a/src/test/java/redis/clients/jedis/SentineledConnectionProviderTest.java b/src/test/java/redis/clients/jedis/SentineledConnectionProviderTest.java index 2270a526fb..68309b15b0 100644 --- a/src/test/java/redis/clients/jedis/SentineledConnectionProviderTest.java +++ b/src/test/java/redis/clients/jedis/SentineledConnectionProviderTest.java @@ -1,6 +1,7 @@ package redis.clients.jedis; import java.util.HashSet; +import java.util.Map; import java.util.Set; import org.apache.commons.pool2.impl.GenericObjectPoolConfig; @@ -8,10 +9,16 @@ import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; import org.junit.jupiter.api.Tag; +import org.junit.jupiter.api.Timeout; import redis.clients.jedis.exceptions.JedisConnectionException; import redis.clients.jedis.exceptions.JedisException; import redis.clients.jedis.providers.SentineledConnectionProvider; +import redis.clients.jedis.util.ReflectionTestUtil; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.hasKey; +import static org.hamcrest.Matchers.sameInstance; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertSame; import static org.junit.jupiter.api.Assertions.assertThrows; @@ -28,6 +35,8 @@ public class SentineledConnectionProviderTest { protected static final HostAndPort sentinel1 = HostAndPorts.getSentinelServers().get(1); protected static final HostAndPort sentinel2 = HostAndPorts.getSentinelServers().get(3); + private static final EndpointConfig primary = HostAndPorts.getRedisEndpoint("standalone2-primary"); + protected Set sentinels = new HashSet<>(); @BeforeEach @@ -51,6 +60,68 @@ public void repeatedSentinelPoolInitialization() { } } + /** + * Ensure that getConnectionMap() does not cause connection leak. (#4323) + */ + @Test + @Timeout( value = 1) + public void getConnectionMapDoesNotCauseConnectionLeak() { + + ConnectionPoolConfig config = new ConnectionPoolConfig(); + config.setMaxTotal(1); + + try (SentineledConnectionProvider sut = new SentineledConnectionProvider(MASTER_NAME, + primary.getClientConfigBuilder().build(), config, sentinels, + DefaultJedisClientConfig.builder().build())) { + + HostAndPort resolvedPrimary = sut.getCurrentMaster(); + ConnectionPool pool = ReflectionTestUtil.getField(sut,"pool"); + assertThat(pool.getNumActive(), equalTo(0)); + + Map cm = sut.getConnectionMap(); + + // exactly one entry for current primary + // and no active connections + assertThat(cm.size(), equalTo(1)); + assertThat(cm, hasKey(resolvedPrimary)); + assertThat(pool.getNumActive(), equalTo(0)); + // primary did not change + assertThat(ReflectionTestUtil.getField(sut,"pool"), sameInstance(pool)); + } + } + + /** + * Ensure that getPrimaryNodesConnectionMap() does not cause connection leak. (#4323) + */ + @Test + @Timeout( value = 1) + public void getPrimaryNodesConnectionMapDoesNotCauseConnectionLeak() { + + ConnectionPoolConfig config = new ConnectionPoolConfig(); + config.setMaxTotal(1); + + try (SentineledConnectionProvider sut = new SentineledConnectionProvider(MASTER_NAME, + primary.getClientConfigBuilder().build(), config, sentinels, + DefaultJedisClientConfig.builder().build())) { + + HostAndPort resolvedPrimary = sut.getCurrentMaster(); + ConnectionPool pool = ReflectionTestUtil.getField(sut,"pool"); + assertThat(pool.getNumActive(), equalTo(0)); + + + Map cm = sut.getPrimaryNodesConnectionMap(); + + // exactly one entry for current primary + // and no active connections + assertThat(cm.size(), equalTo(1)); + assertThat(cm, hasKey(resolvedPrimary)); + assertThat(pool.getNumActive(), equalTo(0)); + // primary did not change + assertThat(ReflectionTestUtil.getField(sut,"pool"), sameInstance(pool)); + } + + } + @Test public void initializeWithNotAvailableSentinelsShouldThrowException() { Set wrongSentinels = new HashSet<>(); diff --git a/src/test/java/redis/clients/jedis/commands/unified/sentinel/SentinelAllKindOfValuesCommandsIT.java b/src/test/java/redis/clients/jedis/commands/unified/sentinel/SentinelAllKindOfValuesCommandsIT.java new file mode 100644 index 0000000000..7ffca8f3f8 --- /dev/null +++ b/src/test/java/redis/clients/jedis/commands/unified/sentinel/SentinelAllKindOfValuesCommandsIT.java @@ -0,0 +1,57 @@ +package redis.clients.jedis.commands.unified.sentinel; + +import org.junit.jupiter.api.extension.RegisterExtension; +import org.junit.jupiter.params.ParameterizedClass; +import org.junit.jupiter.params.provider.MethodSource; +import redis.clients.jedis.DefaultJedisClientConfig; +import redis.clients.jedis.EndpointConfig; +import redis.clients.jedis.HostAndPort; +import redis.clients.jedis.HostAndPorts; +import redis.clients.jedis.JedisClientConfig; +import redis.clients.jedis.JedisSentineled; +import redis.clients.jedis.RedisProtocol; +import redis.clients.jedis.UnifiedJedis; +import redis.clients.jedis.commands.unified.AllKindOfValuesCommandsTestBase; +import redis.clients.jedis.util.EnabledOnCommandCondition; +import redis.clients.jedis.util.RedisVersionCondition; + +import java.util.Arrays; +import java.util.HashSet; +import java.util.Set; + +@ParameterizedClass +@MethodSource("redis.clients.jedis.commands.CommandsTestsParameters#respVersions") +public class SentinelAllKindOfValuesCommandsIT extends AllKindOfValuesCommandsTestBase { + + static final HostAndPort sentinel1 = HostAndPorts.getSentinelServers().get(1); + + static final HostAndPort sentinel2 = HostAndPorts.getSentinelServers().get(3); + + static final Set sentinels = new HashSet<>(Arrays.asList(sentinel1, sentinel2)); + + static final JedisClientConfig sentinelClientConfig = DefaultJedisClientConfig.builder().build(); + + static final EndpointConfig primary = HostAndPorts.getRedisEndpoint("standalone2-primary"); + + @RegisterExtension + public RedisVersionCondition versionCondition = new RedisVersionCondition( + primary.getHostAndPort(), primary.getClientConfigBuilder().build()); + + @RegisterExtension + public EnabledOnCommandCondition enabledOnCommandCondition = new EnabledOnCommandCondition( + primary.getHostAndPort(), primary.getClientConfigBuilder().build()); + + public SentinelAllKindOfValuesCommandsIT(RedisProtocol protocol) { + super(protocol); + } + + @Override + protected UnifiedJedis createTestClient() { + + return JedisSentineled.builder() + .clientConfig(primary.getClientConfigBuilder().protocol(protocol).build()) + .sentinels(sentinels).sentinelClientConfig(sentinelClientConfig).masterName("mymaster") + .build(); + } + +}