feat: support customize dns lookup function #723
Conversation
Since Redis cluster doesn't support hostname at all (redis/redis#2410), it's reasonable to resolve the hostnames to IPs before connecting.
| @@ -117,59 +129,68 @@ class Cluster extends EventEmitter { | |||
| reject(new Error('Redis is already connecting/connected')) | |||
| return | |||
| } | |||
| const epoch = ++this.connectionEpoch | |||
There was a problem hiding this comment.
can overflow (unlikely, but still)
https://github.com/fastify/fastify/blob/master/lib/logger.js#L32-L38 is an example of quick capping of the int so it runs in circles
There was a problem hiding this comment.
permalink at the time of the posting:
| @@ -108,5 +128,6 @@ export const DEFAULT_CLUSTER_OPTIONS: IClusterOptions = { | |||
| retryDelayOnClusterDown: 100, | |||
| retryDelayOnTryAgain: 100, | |||
| slotsRefreshTimeout: 1000, | |||
| slotsRefreshInterval: 5000 | |||
| slotsRefreshInterval: 5000, | |||
| dnsLookup: lookup | |||
There was a problem hiding this comment.
maybe wrap it as a promise here via util.promisify (I dont remember when it became available) or just as a
dnslookup(hostname: string): Promise<string> => {
return new Promise((resolve, reject) => {
lookup(hostname, (err, address) => {
if (err) {
debug('failed to resolve hostname %s to IP: %s', hostname, err.message)
reject(err)
} else {
debug('resolved hostname %s to IP %s', hostname, address)
resolve(address)
}
})
});
},There was a problem hiding this comment.
and then the code would be cleaner in the actual resolve function
# [4.2.0](v4.1.0...v4.2.0) (2018-10-17) ### Features * support customize dns lookup function ([#723](#723)) ([b9c4793](b9c4793)), closes [redis/redis#2410](redis/redis#2410)
|
🎉 This PR is included in version 4.2.0 🎉 The release is available on: Your semantic-release bot 📦🚀 |
|
4.2.0 is released with the "latest" tag instead of "next". Besides that, everything works like a charm. @AVVS |
|
need to make sure that .npmrc is present and I've configured it correctly (ie maybe dist-tag=next) is a wrong way to set it :) but yeah, pretty cool 👍 |
|
Has it been included in Redis 5.0? |
|
@boltzjf It has nothing to do with Redis 5.0. It has been included in ioredis v4.2.0. |
# [4.2.0](redis/ioredis@v4.1.0...v4.2.0) (2018-10-17) ### Features * support customize dns lookup function ([#723](redis/ioredis#723)) ([b9c4793](redis/ioredis@b9c4793)), closes [redis/redis#2410](redis/redis#2410)
Since Redis cluster doesn't support hostname at all (redis/redis#2410),
it's reasonable to resolve the hostnames to IPs before connecting.