Enable Redis tests in cluster mode #1
Conversation
ivanape
commented
Aug 13, 2021
ivanape
commented
- Add support to enable run k6 tests against Redis in cluster mode.
- Add docker-compose to create a test redis cluster.
- Add new k6 example.
- Update README.
ivanape
changed the title
There was a problem hiding this comment.
Hi, thanks for this! And apologies for the slow response, a lot of us were on vacation in August.
I have a concern with the API and it would be great if it could be simplified to use a single constructor. I'm not familiar with Redis clustering, but the docker-compose.yml worked fine, though I'm wondering if that setup should be in the default docker-compose.yml. Having a docker-compose.yml for a single instance is probably overkill since it can easily be started with a docker one-liner, so I guess this is fine.
| // XClusterClient represents the Client constructor (i.e. `new redis.Client()`) and | ||
| // returns a new Redis client object. | ||
| func (r *Redis) XClusterClient(ctxPtr *context.Context, opts *redis.ClusterOptions) interface{} { | ||
| rt := common.GetRuntime(*ctxPtr) | ||
| return common.Bind(rt, &ClusterClient{client: redis.NewClusterClient(opts)}, ctxPtr) | ||
| } |
There was a problem hiding this comment.
I'm not a fan of using a separate constructor and struct for this. You had to duplicate the implementations of Set and Get, and since this extension only exposes the bare minimum Redis commands, it will get very cumbersome if we want to expand it.
We could embed a base struct and avoid that, but I'm wondering if we could use the same Client struct and internally initialize redis.Client or redis.ClusterClient based on a parameter passed to the constructor. Since the only difference seems to be addr vs addrs, could you implement this based on a check if addrs is passed?
| @@ -124,6 +125,9 @@ github.com/golang/protobuf v1.4.0/go.mod h1:jodUvKwWbYaEsadDk5Fwe5c77LiNKVO9IDvq | |||
| github.com/golang/protobuf v1.4.1/go.mod h1:U8fpvMrcmy5pZrNK1lt4xCsGvpyWQ/VVv6QDs8UjoX8= | |||
| github.com/golang/protobuf v1.4.2 h1:+Z5KGCizgyZCbGh1KZqA0fcLLkwbsjIzS4aV2v7wJX0= | |||
| github.com/golang/protobuf v1.4.2/go.mod h1:oDoupMAO8OvCJWAcko0GGGIgR6R6ocIYbsSw735rRwI= | |||
| github.com/golang/protobuf v1.5.0/go.mod h1:FsONVRAS9T7sI+LIUmWTfcYkHO4aIWwzhcaSAoJOfIk= | |||
| github.com/golang/protobuf v1.5.2 h1:ROPKBNFfQgOUMifHyP+KYbvpjbdoFNs+aK7DXlji0Tw= | |||
| github.com/golang/protobuf v1.5.2/go.mod h1:XVQd3VNwM+JqD3oG2Ue2ip4fOMUkwXdXDdiuN0vRsmY= | |||
There was a problem hiding this comment.
I think you might be able to cleanup some of these with go mod tidy.
|
Hi @imiric, Sorry for the delay in responding to this topic. I have come back to it and I think the easiest way to connect to Redis in this scenario, is using the new UniversalClient. I close this PR in favor of the PR: #2. If it's ok with you, we can discuss any question in the new PR. Thank you very much! |
