Skip to content

Reset server-side state when reusing persistent connections#2825

Open
KentarouTakeda wants to merge 4 commits into
phpredis:developfrom
KentarouTakeda:reset-persistent-connection
Open

Reset server-side state when reusing persistent connections#2825
KentarouTakeda wants to merge 4 commits into
phpredis:developfrom
KentarouTakeda:reset-persistent-connection

Conversation

@KentarouTakeda

Copy link
Copy Markdown
Contributor

Problem

When a pooled connection is retrieved via pconnect, server-side state from the previous request—SELECT, WATCH, CLIENT SETNAME, etc.—carries over. This causes reads and writes to go to the wrong database, stale watches to break transactions, and other subtle failures.

Fixes #1920. Related: #428.

Solution

Add the Redis 6.2 RESET command to the redis_sock_check_liveness() pipeline so that all server-side state is cleared on pool retrieval.

  • Pipeline order: RESETAUTHECHO (prepended to the existing liveness check)
  • Pre-6.2 servers: the first -ERR response is cached in ConnectionPool.reset_unsupported; subsequent retrievals skip the command entirely
  • No stats pollution: the errorstats section was introduced in 6.2—the same version that added RESET—so no per-error counter is incremented on older servers; after the flag is cached the command is not sent at all

Why RESET

Approach Drawback
Selective commands (DISCARD+UNWATCH+SELECT 0+…) Must be updated whenever Redis adds new per-connection state
Disconnect and reconnect Negates the benefit of persistent connections (TCP/TLS handshake cost)
RESET Server-authoritative, single command, future-proof

6.2 adoption

RESET was added in Redis 6.2 (February 2021). Major managed services already provide it:

  • AWS ElastiCache — 6.2 available in all regions (November 2021)
  • GCP Memorystore — 6.x ships 6.2.13; default is 7.2 (docs)
  • Azure Cache for Redis — 6.2+ on Enterprise tier (docs)

On pre-6.2 servers, RESET returns -ERR but the connection stays alive. After the flag is cached the additional cost drops to zero.

Changes

  • common.h — add RESP_RESET_CMD constant; add reset_unsupported field to ConnectionPool
  • library.c — send and consume RESET in redis_sock_check_liveness(); pass ConnectionPool * to the function
  • tests/RedisTest.php — three regression tests covering SELECT, WATCH, and CLIENT SETNAME state leaks

Comment thread library.c Outdated
Comment thread library.c Outdated
Comment thread redis.c Outdated
@yatsukhnenko

Copy link
Copy Markdown
Member

@michael-grunder I'm not sure should we use ECHO to check liveness when RESET command is used but in general I'm fine with this PR

KentarouTakeda added a commit to KentarouTakeda/phpredis-phpredis that referenced this pull request Apr 8, 2026
The test used rand() to decide which keys to create, making it
possible for $mkeys to be empty. This caused EXISTS to receive an
empty array, resulting in a sporadic assertion failure:
(false) !== 0

Observed in phpredis#2825 CI and also in an unrelated branch:
- https://github.com/phpredis/phpredis/actions/runs/24132080914
- https://github.com/phpredis/phpredis/actions/runs/23617186872
michael-grunder pushed a commit that referenced this pull request Apr 8, 2026
The test used rand() to decide which keys to create, making it
possible for $mkeys to be empty. This caused EXISTS to receive an
empty array, resulting in a sporadic assertion failure:
(false) !== 0

Observed in #2825 CI and also in an unrelated branch:
- https://github.com/phpredis/phpredis/actions/runs/24132080914
- https://github.com/phpredis/phpredis/actions/runs/23617186872
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Multple persistent connections to the same Redis host with different databases: database number changes

2 participants