jpodwys / cache-service-redis Goto Github PK

Hey,

after some days of production usage in AWS Lambda I notice a very high redis connection count. It seems the connections are not closed on handler termination. The setup is the same as documented in my earlier issue

• Is there any way to close the connections? As I had to set context.callbackWaitsForEmptyEventLoop = false;this might now prevent any connection shutdown logic?
• Does it have to do with backgroundRefresh?

Thanks in advance. If this requires coding work I'm happy to do it and make a PR. =)

Hi John. I would like to be offered the chance to specify that the expiry time, not only as explicit parameter, but to set it upon each request header 'cache-control: max-age=' for example, instead of (or alternativaly to) the configuration module.

//Require superagent and the cache module I want
const RedisModule = require('cache-service-redis');
const redisDefaults = {cacheWhenEmpty: false, expiration: 3600};
let cacheEnv = {redisEnv: 'REDISCLOUD_URL'};
const redisCache = new RedisModule(cacheEnv);
require('superagent-cache')(
superagent,
redisCache,
redisOptions
);

Is it possible?

I think a bump in Redis version has now caused it to emit warning messages when the cache-service-redis connects to it.

These are:

node_redis: Deprecated: The AUTH command contains a "undefined" argument.
This is converted to a "undefined" string now and will return an error from v.3.0 on.

and

node_redis: Warning: Redis server does not require a password, but a password was supplied.

This seems to be caused by this line of code:

https://github.com/jpodwys/cache-service-redis/blob/master/redisCacheModule.js#L351

Which does not check first if there are auth credentials to be passed on to Redis

Hi @jpodwys , I would to thank you for the job in this package. In this direction, I wish to have the latest changes made to "node_redis" v2.8.0+ in the past weeks, supportting "rediss://" protocol usng TLS. It is already included in the master branch of https://github.com/NodeRedis/node_redis , but for some reason it is not been included in your distribution. Maybe you can push some update in order to get this? Also a funtional test about rediss:// would be fine. Regards.

http://localhost:6379
cacheService: Error when getting key key1 from cache of type redis: { name: 'GetException',
message: [TypeError: self.db.get is not a function] }
cacheService: Error when getting key key2 from cache of type redis: { name: 'GetException',
message: [TypeError: self.db.get is not a function] }
redisCacheModule: .set() failed for cache of type redis { name: 'RedisSetException',
message: [TypeError: self.db.setex is not a function] }
redisCacheModule: .set() failed for cache of type redis { name: 'RedisSetException',
message: [TypeError: self.db.setex is not a function] }

We added a second cache key that we need refreshed on our project and used the same callback on both cache stores.

That resulted in the 1st cache to be saved using the 2nds key name and the 2nd cache refresh never getting invoked.

The problem got resolved when we separated the refresh callbacks.

https://github.com/luin/ioredis/wiki/Migrating-from-node_redis

However, if you look at the bottom of https://github.com/NodeRedis/node_redis,

Right now there are two great redis clients around and both have some advantages above each other. We speak about ioredis and node_redis. So after talking to each other about how we could improve in working together we (that is @luin and @BridgeAR) decided to work towards a single library on the long run. But step by step.

So I'm not sure if you'd want to migrate right now. I do need sentinel support soon though so I'll create a fork anyways.

We have a case where we are running out of connections and the line: https://github.com/jpodwys/cache-service-redis/blob/master/redisCacheModule.js#L176 throws an exception that hides the actual error. This throws an exception when result.length is used before checking to see if err is set or response is not set. You have code elsewhere that checks for if (!err && response) This would help solve this problem.