Skip to content

fix(limit-count): redis-sentinel keepalive pools must not be shared across database/credentials#13553

Open
nic-6443 wants to merge 1 commit into
apache:masterfrom
nic-6443:fix/sentinel-pool-bump-connector
Open

fix(limit-count): redis-sentinel keepalive pools must not be shared across database/credentials#13553
nic-6443 wants to merge 1 commit into
apache:masterfrom
nic-6443:fix/sentinel-pool-bump-connector

Conversation

@nic-6443

Copy link
Copy Markdown
Member

Description

Fixes #13454

Sentinel-mode counterpart of #13516. The redis-sentinel policy goes through api7-lua-resty-redis-connector, which skips AUTH/SELECT on connections reused from the keepalive pool while pooling by the default host:port key — so two routes pointing at the same sentinel-managed master with different redis_database or credentials cross-contaminate exactly like the plain-redis case: limit counters land in the wrong database.

This bumps the connector to 0.13.0 (api7/lua-resty-redis-connector#9), which derives the default pool name from everything the connection gets bound to at handshake time — resolved address, database, a credentials digest, and TLS options — while an explicit connection_options.pool still wins. Building the name from the resolved address keeps failover correct: a new master gets a new pool instead of reusing connections to the old one.

The regression test mirrors the plain-redis one: two routes on the same sentinel master with redis_database 1 and 2 must keep their counters in their own databases. Without the bump it fails with both keys landing in db 1. The test helper now also flushes the sentinel master (port 6479) so counters don't survive CI re-runs.

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible

…and credentials

Bump api7-lua-resty-redis-connector to 0.13.0: the connector skips
AUTH/SELECT on connections reused from the pool, so the default
host:port pool cross-contaminates configs with different databases or
credentials, the sentinel-mode counterpart of the plain-redis fix.
0.13.0 derives the default pool name from the resolved address,
database, credentials digest and TLS options.

Also flush the sentinel master in the test helper so its counters do
not survive CI re-runs.
Copilot AI review requested due to automatic review settings June 12, 2026 09:00
@dosubot dosubot Bot added size:L This PR changes 100-499 lines, ignoring generated files. bug Something isn't working labels Jun 12, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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

Labels

bug Something isn't working size:L This PR changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

bug: module apisix.utils.redis - Redis connection reuse issue

2 participants