Missing connection with endpoint fallback in cluster connection

[jabolina]

Version

4.4.4

Context

This seems a corner-case, and likely people won't usually hit. By falling back to the endpoints given during configuration if a slot is not found:

vertx-redis-client/src/main/java/io/vertx/redis/client/impl/RedisClusterConnection.java

Lines 508 to 511 in a5a54f7

	// if we haven't got config for this slot, try any connection
	if (endpoints == null || endpoints.length == 0) {
	return connectOptions.getEndpoint();
	}

The URI might contain some query parameters which cause the command to fail on:

vertx-redis-client/src/main/java/io/vertx/redis/client/impl/RedisClusterConnection.java

Lines 255 to 260 in a5a54f7

	final PooledRedisConnection connection = connections.get(endpoint);
	if (connection == null) {
	handler.handle(Future.failedFuture("Missing connection to: " + endpoint));
	return;
	}

More concretely, after executing the CLUSTER SLOTS command, the connections map to the endpoints returned, which has the format rediss://127.0.0.1:11222 (with SSL).

vertx-redis-client/src/main/java/io/vertx/redis/client/impl/RedisClusterClient.java

Lines 177 to 179 in a5a54f7

	synchronized (connections) {
	connections.put(endpoint, cconn);
	}

If (for some unusual) reason the slot is not found, falling back to the configured endpoint might not be reliable, as the URI is not necessarily the same, e.g., rediss://127.0.0.1:11222?verifyPeer=NONE. Searching the connections map returns null for the endpoint.

Do you have a reproducer?

I am unsure of other ways to fall back to the configuration URI. So, sadly, I don't have a reproducer for this one. I believe this is really unlikely to happen, but I thought it might be worth reporting it.

[tsegismont]

@Ladicek would you mind taking care of this one?

[Ladicek]

Just for the record, I didn't forget about this. This is basically about canonicalizing the key to the connection pool map. I'd like to improve the code related to connection pooling more thoroughly [1], and will address this at that point.

[1] For example, there are some validations for connection pool configuration that assume we have a single connection pool, which we don't. We have one connection pool for each connection string. There's also this issue, which I think we should address.