search

h0x91b / redis-fast-driver

78 stars 13 forks source link

Connection Timeout #23

Open STRML opened 6 years ago

STRML commented 6 years ago

Would be great to have a feature to set a connectTimeout and have the native addon give up on connecting after that time. In the wild I've seen a complete hang after the first reconnect from which this adapter never recovers. Unfortunately I can't reproduce but it seems to happen if redis crashes unexpectedly and doesn't tear down the socket correctly.

I've implemented it partially on the JS side but this doesn't seem like the right approach - if the remote is simply slow to connect, the onConnect callback from the lagging connection will fire and you might get multiple connect events. So it seems best to do this in the native addon.

shuckc commented 5 years ago

hiredis supports this via. redisConnectWithTimeout so I will prepare a patch.

h0x91b commented 5 years ago

Thanks

shuckc commented 5 years ago

Ah - redisConnectWithTimeout() doesn't use libuv, it's a plain old blocking/synchronous API in hiredis, exactly as this reporter figured out too https://github.com/redis/hiredis/pull/290 So it's not that easy, since libuv has no built in api for adding a timeout to an async socket connection attempt. There is a 3rd party workaround on the libuv help issue list https://github.com/libuv/help/issues/54 this would need tidying up and incorporating into hiredis first before we could use it. Similar discussion here https://github.com/joyent/libuv/issues/1415

h0x91b commented 5 years ago

Maybe just to PING/PONG every X minutes?

пт, 21 июн. 2019 г., 17:50 Chris Shucksmith notifications@github.com:

Ah - redisConnectWithTimeout() doesn't use libuv, it's a plain old blocking/synchronous API in hiredis, exactly as this reporter figured out too redis/hiredis#290 https://github.com/redis/hiredis/pull/290 So it's not that easy, since libuv has no built in api for adding a timeout to an async socket connection attempt. There is a 3rd party workaround on the libuv help issue list libuv/help#54 https://github.com/libuv/help/issues/54 this would need tidying up and incorporating into hiredis first before we could use it.

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/h0x91b/redis-fast-driver/issues/23?email_source=notifications&email_token=AAGFYGZYC6YMY6V27F42DALP3TTB3A5CNFSM4FPCMYU2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODYIVXVA#issuecomment-504454100, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGFYGZUNOT2HRJ63RMGYA3P3TTB3ANCNFSM4FPCMYUQ .