Skip to content

Commit

Permalink
fix pool metrics for tainted connections
Browse files Browse the repository at this point in the history
In case of a tainted connection, the metrics were not ended properly.
This commit fixes that while retaining idempotency of `PooledRedisConnection.close()`.
  • Loading branch information
Ladicek committed Jan 14, 2025
1 parent 66afb84 commit 15915c5
Show file tree
Hide file tree
Showing 2 changed files with 34 additions and 0 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.vertx.redis.client.Response;

import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;

/**
* A pooled Redis connection
Expand All @@ -20,6 +21,7 @@ public class PooledRedisConnection implements RedisConnection {
private final RedisConnectionInternal connection;
private final PoolMetrics metrics;
private final Object metric;
private final AtomicBoolean ended = new AtomicBoolean();

public PooledRedisConnection(Lease<RedisConnectionInternal> lease, PoolMetrics<?> poolMetrics, Object metric) {
this.lease = lease;
Expand Down Expand Up @@ -88,7 +90,9 @@ public RedisConnection endHandler(@Nullable Handler<Void> endHandler) {
public Future<Void> close() {
if (connection.reset()) {
lease.recycle();
}

if (ended.compareAndSet(false, true)) {
if (metrics != null) {
metrics.end(metric, true);
}
Expand Down
30 changes: 30 additions & 0 deletions src/test/java/io/vertx/redis/client/test/RedisPoolMetricsTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,36 @@ public void simpleTest(TestContext should) {
});
}

@Test
public void taintedConnection(TestContext test) {
Async async = test.async();

Redis client = Redis.createClient(rule.vertx(), new RedisOptions().setConnectionString(redis.getRedisUri()));
client.connect()
.compose(conn -> {
test.assertEquals(0, getMetrics().numberOfWaitingTasks());
test.assertEquals(1, getMetrics().numberOfRunningTasks());

return conn.send(Request.cmd(Command.SELECT).arg(7)) // taints the connection
.compose(response -> {
test.assertEquals(0, getMetrics().numberOfWaitingTasks());
test.assertEquals(1, getMetrics().numberOfRunningTasks());

return conn.close();
}).onComplete(test.asyncAssertSuccess(ignored -> {
test.assertEquals(0, getMetrics().numberOfWaitingTasks());
test.assertEquals(0, getMetrics().numberOfRunningTasks());
}));
})
.map(ignored -> {
client.close();
return null;
})
.onComplete(test.asyncAssertSuccess(ignored -> {
async.complete();
}));
}

@Test
public void testLifecycle(TestContext should) {
final Async test = should.async();
Expand Down

0 comments on commit 15915c5

Please sign in to comment.