node-redis icon indicating copy to clipboard operation
node-redis copied to clipboard

v4 can't quit you - async client usage leaving idle connections?

Open ChrisArchitect opened this issue 2 years ago • 3 comments

Maybe I don't understand async/await correctly but can someone help or verify the way to createClient and send commands with async/await that ensures connections actually quit so that all these idle connections aren't made on a recurring script run?

I updated this code from v3 to v4.0.4 and keep getting increasing open(but idle?!) connections that won't go away. (it's on heroku redis if that matters)

near top of module createClient to be used later as a pool connection:

const rclient = require("redis").createClient({
	url: process.env.REDIS_TLS_URL,
	socket: {
		tls: true,
		rejectUnauthorized: false
	},
	legacyMode: true
});
rclient.on('error', (err) => {});

Then later on run a basic SETNX....

  (async () => {
	  try {
		await rclient.connect();
		await rclient.SETNX(...,function(err, dne) {
			if (err) {
				console.log('SETNX problem: '+err);
			} else if (dne) {
				//stuff											
			}
		});
		await rclient.quit();
	} catch (e) {
		console.error(e);
        }
  })();

I include that await quit() there and expect it to close the connection after command completes..... but I keep getting mounting idle connections? (Why don't they just clear out after expiry time?) I just want to send my command and quit! Any direction or verification that this is the correct usage appreciated

ChrisArchitect avatar Mar 19 '22 03:03 ChrisArchitect

on a bit further inspection of the CLIENT LIST, I see the connections are idling after use as they should and then going away. Great. But then they seem to 'come back'?! IS this something to do with redis server defaulting to reconnect? Or something to do with the way reconnect strategy is implemented? I haven't defined any reconnect strategy but maybe I should explicitly turn it off? I don't need old connections to come back.

What happens if I make the connection idle time something long -- will the next time code tries to createClient from the same port/IP it just reopen/use that idle connection?

ChrisArchitect avatar Mar 25 '22 17:03 ChrisArchitect

could this be related to setting legacyMode to true? Should legacy mode be false if using async/await?

andrewrlee avatar Apr 28 '22 14:04 andrewrlee

Thank you for replying. I cannot test currently since I only had a short window of using the code in action and ended up just resetting connections constantly which was a drag.

legacyMode was so code could be used with methods like SETNX instead of refactoring to set('NX'...) etc. So perhaps that is a problem. But meanwhile, are you saying the general setup of the async code looks correct?

ChrisArchitect avatar May 01 '22 20:05 ChrisArchitect

Revisiting this code, still causing problems with connections. Still not sure if async code is correct for legacyMode or whatever.

Saw someone mention using a different quit call -- so changed the above quit to await rclient.v4.quit() -- and whatdya know, excess connections problem went away!! Weird!

But then shortly after, got a Socket already opened error the next time connect() was run. What gives? Does legacyMode quit() not work, but v4 does, but then it's all confused from the createClient point to the quit if mixing all different methods.

Can anyone confirm any differences/bugs between quit() and v4.quit? or direct me on rewriting my legacy SETNX call there so all redis calls are in line with v4?

I must say it's annoying that all old code with v3 worked without any issues at all and moving to v4 is nothing but these problems.

ChrisArchitect avatar Mar 17 '23 21:03 ChrisArchitect

client.quit is indeed broken in legacy mode, currently in legacy mode only client.v4.quit() works.

Regarding how to use SETNX:

// using SETNX:
await client.SETNX('key', 10, 'value');

// using SET with NX
await client.SET('key', 'value', {
  NX: 10
}); // using SET

leibale avatar Mar 18 '23 23:03 leibale

thanks for that. Kinda crazy about legacy mode thing. Granted I should have put the time in to just rewrite the code to v4, more up-to-date JS, but I also shouldn't have had to pull so much hair out about the mysterious connection problems etc!

Have now rewritten the above to something more along the lines of:

async () => {
    try {
        await client.connect();
        const dne=await client.set(key,value,{
            NX: true
        });
        if (dne){ //stuff }
        await client.quit();
  } catch (e) {
      console.error(e);
}

(for anyone that might be searching for help)

So far, so good. No idle connections reappearing, redis keys being set etc. fingers crossed

ChrisArchitect avatar Mar 19 '23 02:03 ChrisArchitect

@ChrisArchitect I would advise creating a client and reusing it instead of creating and closing a client per request

leibale avatar Mar 19 '23 06:03 leibale