Add SELECT command to RedisCluster for Valkey 9+ - #2902
Open
uintaam wants to merge 9 commits into
Open
Conversation
cluster_init_cache() created seed sockets without copying user/pass from the cluster flags, unlike cluster_init_seeds(). If a keyspace re-map was triggered after loading slots from the cache (e.g. after a failover), mapping would fail against auth-enabled clusters. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Valkey 9.0 supports numbered databases in cluster mode (cluster-databases config). Database selection is per-connection state, so a cluster client must (re)issue SELECT on every node connection. phpredis already does exactly that for any socket with a nonzero dbNumber via the connect state machine in redis_sock_server_open() and the reconnect path in redis_check_eof() -- cluster sockets simply never had dbNumber set. This adds: * An eighth RedisCluster constructor argument (int $database = 0), plus a redis.clusters.database INI key for named clusters. * RedisCluster::select() and RedisCluster::getdbnum(). select() eagerly validates the database against a single master (so servers without cluster database support fail cleanly with the error available from getlasterror()), then propagates lazily: other sockets are disconnected and reselect the new database the next time they are used. * Propagation of the configured database onto every node socket at creation (cluster_node_create, cluster_init_seeds, cluster_init_cache), which covers seeds, MOVED/ASK-discovered nodes, replicas and remaps. * Database-segregated persistent connections: the pool key and the non-pooled persistent id gain a :db<N> suffix when the database is nonzero, so a pooled stream left on db N can never be handed to a client expecting another database. Keys for db 0 are unchanged. This also fixes the same latent leak for standalone Redis when using the 'database' option with persistent connections. * Clearer errors when a backend rejects SELECT: construct and first-command failures now surface the server error (e.g. "SELECT is not allowed in cluster mode") instead of generic communication errors. When the database is 0 (the default) no SELECT is ever sent and pool keys are byte-identical, so behavior with Redis, KeyDB and Valkey < 9 clusters is completely unchanged. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Accept a database=N query parameter in session.save_path for the rediscluster session handler, mirroring the standalone handler. Requires a server with database support in cluster mode (Valkey >= 9.0 with cluster-databases > 1). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Covers both directions: gated on a Valkey 9 cluster with databases enabled, verify select(), the constructor argument, persistent pool isolation, reconnect and the session handler save_path. On servers without support (Redis, KeyDB, Valkey < 9) verify that selecting a nonzero database fails cleanly, leaves the connection usable and that constructing with one raises a clear exception. The standalone testPersistentDatabasePoolIsolation test fails without the pool key fix, where a stream left on database 2 is handed back to a client that asked for database 0. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Add a make-cluster.sh -d option to spawn a cluster with numbered databases, and pin one CI leg to Valkey 9.1.1 started with cluster-databases 16 so the feature is actually exercised. The remaining valkey legs stay on 8.1.3 to keep covering servers without database support in cluster mode. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
detectValkey() compared the 'executable' info field against the literal string 'valkey', but that field holds a full path, so is_valkey was never true and every minValkeyVersionCheck() gated test silently skipped. Match on server_name/valkey_version instead, which turns the existing Valkey 9 geo polygon tests back on (both pass), along with the new cluster database tests. Also handle both reply shapes when reading cluster-databases from CONFIG GET. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Without this a stale error from an earlier command could make a write failure look like the server rejecting the database. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Valley 9 added support for database when in cluster mode. This is an attempt to add db select support.
#2715