node-server-sdk icon indicating copy to clipboard operation
node-server-sdk copied to clipboard

Support Redis Cluster store via ioredis

Open seanxiesx opened this issue 7 years ago • 7 comments
trafficstars

https://github.com/luin/ioredis

seanxiesx avatar Sep 20 '18 06:09 seanxiesx

@eli-darkly I'm going to take a stab at swapping out redis with ioredis. We need this as well.

rmanalan avatar Nov 27 '18 22:11 rmanalan

@rmanalan We hadn't had a chance to look into this yet, so I'd certainly be curious whether you run into any problems. Thanks.

eli-darkly avatar Nov 27 '18 22:11 eli-darkly

@rmanalan By the way, as of v5.6.1 there is an optional new helper mechanism for building a feature store integration that makes things quite a bit simpler. You can take a look at https://github.com/launchdarkly/node-dynamodb-store for an example of how it can be used. In that example, it specifies the Node SDK as a peer dependency so it does actually require that v>=5.6.1 be present, but if you want to be more backward-compatible you could also just pull in the Node SDK code as a regular dependency so as to use the helper stuff from it.

eli-darkly avatar Nov 27 '18 22:11 eli-darkly

@eli-darkly I have a working version using ioredis, however, I need to make it generic so that it doesn't break existing users. Also, I introduced a lock mechanism so that in clustered multi-process environments, you only have one process that writes to redis at a time. This might eliminate the need to run ld-relay in large clustered environments. I'll work on a PR soon.

rmanalan avatar Nov 28 '18 01:11 rmanalan

@rmanalan I'm not sure what you mean by "I need to make it generic"; could you clarify?

eli-darkly avatar Nov 28 '18 01:11 eli-darkly

@eli-darkly our service uses Electrolyte for DI. Redis in our env is defined as a component. So, my version of the redis feature store doesn't actually connect to redis... it just uses the already instantiated component. So, what I mean by making it generic is I need to make sure that you can either pass in an already instantiated redis client or the standard connection params that the original redisFeatureStore supports. I think this is a good addition anyway since in most environments devs might want to use an existing client.

rmanalan avatar Nov 28 '18 02:11 rmanalan

@rmanalan Got it. Yes, we've generally taken that approach in our other SDKs, but I guess we didn't think of it in the Node one.

eli-darkly avatar Nov 28 '18 02:11 eli-darkly

@seanxiesx

The new version of the SDK, v8, supports ioredis.

The new version has a new location and package.

The new version is located here: https://github.com/launchdarkly/js-core/tree/main/packages/sdk/server-node It also has a new package name, @launchdarkly/node-server-sdk.

The redis package is also moved and renamed.

Github: https://github.com/launchdarkly/js-core/tree/main/packages/store/node-server-sdk-redis Npm: https://www.npmjs.com/package/@launchdarkly/node-server-sdk-redis

Thank you, Ryan

kinyoklion avatar Jun 29 '23 22:06 kinyoklion

This issue is marked as stale because it has been open for 30 days without activity. Remove the stale label or comment, or this will be closed in 7 days.

github-actions[bot] avatar Jul 30 '23 01:07 github-actions[bot]