Visitar URL original
RedisCluster leaks node connections during failover remapping · Issue #2949 · phpredis/phpredis · GitHub
Skip to content

RedisCluster leaks node connections during failover remapping #2949

Description

@ebubekiryigit

Expected behaviour

Rebuilding the cluster topology after a failover should not orphan existing node connections. Non-persistent connections should be released; clean persistent connections may follow the existing pooling policy.

Actual behaviour

On unmodified develop, node connections accumulate across failovers, including with persistent connections disabled. Calling RedisCluster::close() only closes the currently mapped connections.

With three masters and three replicas, I observed the following counts for connections belonging to the same client, identified by a unique CLIENT SETNAME:

Before failover:       3
After first failover:  6
After second failover: 9
After third failover:  12
After close():        9

The leak reproduction uses ordinary commands, without WATCH, MULTI, or pipeline mode.

I'm seeing this behaviour on

  • OS: Linux x86_64, running locally in Docker
  • Redis: 8.8.0
  • PHP: 8.5.2
  • phpredis: develop at 146ec813ea7ca85c9e3a7cff2caf977096e16acf
  • redis.clusters.cache_slots=0
  • RedisCluster persistent constructor argument: false

Steps to reproduce

Use a disposable local cluster with three masters and one replica per master.

  1. Create one non-persistent RedisCluster object and keep it in the same PHP process throughout the test.
  2. Set a test key, and open a connection to each master using CLIENT SETNAME with the same unique name. Count matching connections with CLIENT LIST on all six nodes.
  3. Promote the replica serving the test key using CLUSTER FAILOVER TAKEOVER.
  4. Wait for the slot owner to converge across the nodes, then call GET for the test key through the original object. This triggers the MOVED handling and topology remap.
  5. Set the client name on all currently mapped master connections again and recount. Repeat for three failovers, waiting for replication to be ready before each promotion.
  6. Call close() on the original object and recount before the PHP process exits.

Suspected cause and implementation question

cluster_update_slot() calls cluster_map_keyspace() when MOVED points to a known replica. cluster_map_slots() then clears c->nodes; its node destructor frees the socket structures without disconnecting their streams. The old connections are no longer reachable through the cluster object.

I tested disconnecting before the remap locally. It resolves the leak, but needs care with WATCH: normal disconnect can return a WATCH-active persistent stream to the pool for another client, while force-closing it loses the original client's WATCH state. Neither should silently affect subsequent transactions.

Before proposing a patch, would rejecting this full remap with an exception while WATCH is active be the preferred behaviour? I would like to keep the fix narrow and preserve existing transaction guarantees.

Related: #2025 and #2655.

I've checked

  • I did not find an existing issue describing this connection leak.
  • Reproduced on the unmodified develop commit listed above.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions