search

next-boost / redis-cache

This is the cache plugin for running next-boost on a cluster
6 stars 2 forks source link

Added support for redis cluster mode via ioredis #2

Closed prkirby closed 2 years ago

prkirby commented 2 years ago

Add support for passing an ioredis cluster config object to Cache constructor

Update count function to use a promise array instead of mget as mget is not supported for clusters when keys are stored in different slots

For testing purposes, you can use this docker container for a redis cluster: https://github.com/Grokzen/docker-redis-cluster

rjyo commented 2 years ago

I think it might be better to just let the users pass the Redis client instance to the cache. There are still much more options for initializing Redis that we can't cover in this kind of interface.

export interface CacheOptions {
  uri?: string
  redis?: Redis.Redis | Redis.Cluster
  ttl?: number
  tbd?: number
}
prkirby commented 2 years ago

Good call, let me make whip that up quick.

prkirby commented 2 years ago

@rjyo Ok that should be all set, let me know if theres anything else that needs clean up.

prkirby commented 2 years ago

@rjyo Do you think this is something that could get merged? Our team needs this feature for a release we are hoping to make in the near future.

electriccode commented 2 years ago

@rjyo If it's all good can we go ahead with merging this?

prkirby commented 2 years ago

Added in Flatted as I was getting Circular JSON Error when using this code in my repo, due to init() in adapter.ts calling JSON.stringify() on options. Since passing in a Redis instance is a circular object, this was throwing UnhandledPromiseRejectionWarning: TypeError: Converting circular structure to JSON

piyu-sh commented 2 years ago

@rjyo Do you think this can be merged ?

electriccode commented 2 years ago

@rjyo do you have some time to look at this? For now we're using npm-patch-package to have this up and running but we don't want to hit product with monkey patched npm package. Would be great if we can close this one.

prkirby commented 2 years ago

@rjyo let me know if there is anything you want changed / updated.

rjyo commented 2 years ago

Sorry for being late on this issue. I actually can't get the test running in the pipeline.

prkirby commented 2 years ago

@rjyo For the test, you need to have a redis instance that supports clustering. Here is the Docker container that I used locally, not sure if that will help: https://github.com/Grokzen/docker-redis-cluster