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

Add NAT support for redis cluster #636

Closed
wants to merge 1 commit into from
Closed

Conversation

heri16
Copy link

@heri16 heri16 commented Jun 7, 2018

Add support for redis clusters than are behind Network Address Translation (NAT).
This is useful for scenarios like this: https://docs.aws.amazon.com/AmazonElastiCache/latest/red-ug/accessing-elasticache.html#access-from-outside-aws

@jcstanaway
Copy link

This only works if natRemap contains the entire set of nodes in the cluster. In general, you don't need to specify all of the nodes in the nodeList argument when you do a new Redis.Cluster() and ioredis will dynamically learn the rest of the nodes in the cluster. This solution essentially requires the client to be fully configured with all of the nodes.

Further, this solution does not allow for the cluster to dynamically change.

  1. The size of the cluster can not increase unless natRemap is originally set to include all possible nodes that may exist in the future.

  2. The NAT mapping is static. If a node is ever restarted with a different IP address, the natRemap will need to be updated and the client restarted.

For rather static deployments, this appears to work, but the above limitations should be highlighted as this won't work in all deployments.

@heri16
Copy link
Author

heri16 commented Jun 11, 2018

@ccs018 Yes, I am well aware of these limitations. ElastiCache ensures that both the DNS name and the IP address of the cache node remain the same when cache nodes are recovered in case of failure.
See here

Adding shards while online would require a RegEx-based implementation that maps all matching auto-generated internal-DNS hostnames to the Natted ip address. The node hostnames on AWS ElastiCache can be predicted ahead of time, and this is likely the same case for other automated deployment tools also. [xxxx-cache-0001-001.xxxx-cache.xx.xxxx1.cache.amazonaws.com] With the right regexp pattern, this addresses point 1 so that we can include all possible nodes that may exist in the future, including nodes spanning across availability zones.

To address point 2, the cluster should be using static internal hostnames instead of static internal ip addresses. This is already the case for AWS ElastiCache. If the node is ever restarted, internal dns should be updated to reflect the new IP address, without having to update natRemap or restarting the client.

Would we prefer a regex implementation? Alternatively, the configuration could be externalized to a JSON file with fs.watch() hot-reloading.

@heri16
Copy link
Author

heri16 commented Jun 11, 2018

Just in case others are wondering why-NAT, this is to mitigate cold-start-times on AWS Lambda, in particular the slow performance when initializing new VPC-connection. This means an extra long time for ioredis to successfully connect to redis cluster because the VPC network interface requires some time to initialize and be ready.

See article

@stale
Copy link

stale bot commented Jul 11, 2018

This issue has been automatically marked as stale because it has not had recent activity. It will be closed after 7 days if no further activity occurs, but feel free to re-open a closed issue if needed.

@stale stale bot added the wontfix label Jul 11, 2018
@luin luin added pinned and removed wontfix labels Jul 12, 2018
@luin luin self-assigned this Oct 8, 2018
@luin
Copy link
Collaborator

luin commented Dec 12, 2018

Sorry for the (really) late response. This issue has been resolved in #758. Let me know if there's any problem.

@luin luin closed this Dec 12, 2018
@heri16
Copy link
Author

heri16 commented Apr 9, 2019

Will look into it. Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants