-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
multiple concurrent connect if error happened when connect #2320
Comments
Can you please try to upgrade to 4.4.0? Might be a duplicate of #2010 |
Feel free to reopen if the issue still exists |
Hi! I'm using the latest version of node-redis (4.6.4), and this bug is still present.
The code is very simple and is similar to @F3n67u . The flow is:
|
@orinciog can you please share your code so I'll be able to reproduce the issue? |
@leibale Thank you for you very quick answer. I was using a simple code, just like the above code. Testing it on different scenarios to show you, I realized that the problems were introduced by the istio sidecar proxy. The only scenario in which I had problems which reconnection was when I deployed redis client in kubernetes with istio sidecar proxy in front of client. With istio, the client had a lot of reconnection problems:
After I disabled the istio sidecar, everything was ok, so the problem is on istio proxy side. Thank you again for your quick answer. |
You can reproduce the issue by...and don't ask me how I know this...pointing Redis at a RabbitMQ server. Obviously the underlying cause is the connection being broken but I don't think the library should enter an infinite reconnect loop when this occurs. Example with docker compose: version: "3.9"
services:
rabbitmq:
image: rabbitmq:3-management-alpine
ports:
- 5672:5672
- 15672:15672 import { createClient } from "redis";
async function run() {
const url = "redis://guest:guest@localhost:5672"
const client = createClient({ url });
client.on("error", () => { console.error("Redis Client Error"); });
client.connect();
}
run(); |
I created an Azure redis, it allows access only via SSL. I misconfigure my protocol to
redis://
(connect will trigger error) and found that:How to reproduce
Actual behavior
from the log, we can see that:
Expected behavior
Environment:
My investigation
For now, I only investigated the multiple concurrent connect issue. I tried but didn't figure out why "Cannot read properties of undefined (reading 'destroy')" happen.
In
RedisSocket.#connect()
, when an error happened when connect:error
event is emitted, and the error event listener of RedisSocket(#onSocketError
) invokes theRedisSocket.#conncect
So every time,
RedisSocket.#conncect
in#onSocketError
creates 1 more new connect and concurrent connect become more and more.node-redis/packages/client/lib/client/socket.ts
Line 141 in 2a5dc75
node-redis/packages/client/lib/client/socket.ts
Lines 202 to 209 in 2a5dc75
I think we can skip
RedisSocket.#conncect
in#onSocketError
if the socket is still in the connecting phase.The text was updated successfully, but these errors were encountered: