Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SentinelIterator iterator grows indefinitely with unwanted ips #798

Closed
jschweik opened this issue Feb 18, 2019 · 1 comment
Closed

SentinelIterator iterator grows indefinitely with unwanted ips #798

jschweik opened this issue Feb 18, 2019 · 1 comment
Labels

Comments

@jschweik
Copy link
Contributor

Description:

We're using an environment with only dynamic ips. So we have created a kubernetes service that will always resolve to a healthy sentinel: redis.sentinel. When we set the sentinels parameter to this host, all works fine as an ip is resolved and the connection proceeds.

When ioredis connects to the master though, it asks the master for its sentinel list and proceeds to add all those ips to the sentinel list as well: redis.sentinel, ip1, ip2, ip3. This is ok for the moment as those are the valid ips of the current sentinels. But when those pods die, new ips (ip4-ip6) are added but the old ips (ip1-ip3) that will never be reused are still in the list: redis.sentinel, ip1, ip2, ip3, ip4, ip5, ip6.

Connections continue to work as if ip1-3 are tried, they would just fail and the next sentinel in the list is tried until a working one is found. Over time and many updates for long running clients, eventually this list becomes more like: redis.sentinel, ip1, ... ip100 and beyond.

I understand the reasoning behind adding the new ips and not removing the old ones as the old ones might just be sentinels that are having an outage and will eventually come back up so there is no good knowledge of when to remove old ips.

But for some cases like ours or a list of known static ips, we know the only list of sentinel values we want to use and don't want to add any other possible sentinels to that list. Not checking for any other values and not adding them to the list would be a performance improvement for us.

Suggested Change:

An option updateSentinels whose default would be true with the current behavior to update the sentinel list as new sentinels are discovered, but could be set to false where no new values would ever be added to the sentinel list.

The behavior of iterating through the list looking for good values would remain intact, it would just be the list itself that would never get any additional values if updateSentinels were set to false.

This change would mainly be to the lib/connectors/SentinelConnector/index.ts - updateSentinels method, wrapping it with a check for the value of updateSentinels, only running its content if the value if true.

@luin luin closed this as completed in 50a9db7 Mar 12, 2019
ioredis-robot pushed a commit that referenced this issue Mar 12, 2019
# [4.7.0](v4.6.3...v4.7.0) (2019-03-12)

### Features

* add updateSentinels option to control new sentinel values being added to the original list ([#814](#814)) ([50a9db7](50a9db7)), closes [#798](#798)
@ioredis-robot
Copy link
Collaborator

🎉 This issue has been resolved in version 4.7.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

janus-dev87 added a commit to janus-dev87/ioredis-work that referenced this issue Mar 1, 2024
# [4.7.0](redis/ioredis@v4.6.3...v4.7.0) (2019-03-12)

### Features

* add updateSentinels option to control new sentinel values being added to the original list ([#814](redis/ioredis#814)) ([50a9db7](redis/ioredis@50a9db7)), closes [#798](redis/ioredis#798)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants