search

zalando / skipper

An HTTP router and reverse proxy for service composition, including use cases like Kubernetes Ingress
https://opensource.zalando.com/skipper/
Other
3.12k stars 351 forks source link

Make Redis Config Reusable #1172

Open alimakarfi opened 5 years ago

alimakarfi commented 5 years ago

Skipper can be configured to do rate-limiting using a Redis Ring. Currently the Redis client is well encapsulated in the rate-limit package and well hidden from the outside world. Unfortunately, this makes it impossible to reuse the already existing Redis Ring configuration in other places.

There are at least two solutions: A quick win solution could just expose the ratelimit.newRing and make the ring and corresponding metrics publicly accessible. A more sophisticated solution could decouple the Redis Ring from the ratelimitting feature/ package and introduce some kind of central storage service.

Both solutions would allow to use the globally configured Redis Ring also for other use cases in addition to the standard rate-limiting. For instance, custom filter implementations might want to do some operations based on key-value pairs stored in the same Redis.

Once it is aligned we could work on it.

aryszka commented 5 years ago

From the two options above, I prefer the latter one, moving it into its own package.

On top of that, I would also consider a third approach, to see if it is possible to abstract away redis as implementation to something that reflects only the general purpose. In case of the rate limiters, the purpose is that the skipper instances in a cluster know about each other, and they can exchange their state with each other. I don't have a good name, earlier I called it 'swarm', but it may not be the best. Naming aside, we already have two kinds of such clustering of skippers, one based on Redis, another based on the SWIM protocol. What do you think?

szuecs commented 5 years ago

@alimakarfi can you reply to @aryszka, thanks.

I am also more for the interface definition. I am not sure if in case of clusterRatelimit it can be easily done, because we use redis commands as algorithm.