ratelimit icon indicating copy to clipboard operation
ratelimit copied to clipboard

Using Redis in socket mode no longer works

Open moderation opened this issue 4 years ago • 3 comments

I had a demo of ratelimit running in a Kubernetes cluster at the end of 2019. For performance reasons I had ratelimit connect to a Redis instance over a socket shared on a ConfigMap volume. Having the ratelimit -> Redis connection not use a network at all is great for performance.

Turns out https://github.com/envoyproxy/ratelimit/commit/3ec0f5f3a660ab3ce1f6e16f3823e18dd1ceebf7#diff-ab160e3a4ff5cb5fd488f666c5266fdd disabled the default Unix socket usage by hardcoding the use of TCP.

It is not possible to launch ratelimit with REDIS_SOCKET_TYPE=unix even though it is the 'default' value. See an example below:

env USE_STATSD=false LOG_LEVEL=debug REDIS_URL=/tmp/redis.sock REDIS_SOCKET_TYPE=unix RUNTIME_ROOT=examples/ratelimit/config/ RUNTIME_SUBDIRECTORY=. bin/ratelimit
WARN[0000] statsd is not in use
DEBU[0000] runtime changed. loading new snapshot at examples/ratelimit/config
DEBU[0000] runtime: processing examples/ratelimit/config
DEBU[0000] runtime: processing examples/ratelimit/config/config.yaml
DEBU[0000] runtime: adding key=config.yaml value=---
domain: mongo_cps
descriptors:
  - key: database
    value: users
    rate_limit:
      unit: second
      requests_per_unit: 500

  - key: database
    value: default
    rate_limit:
      unit: second
      requests_per_unit: 500
 uint=false
WARN[0000] connecting to redis on /tmp/redis.sock with pool size 10
panic: dial tcp: address /tmp/redis.sock: missing port in address

goroutine 1 [running]:
github.com/envoyproxy/ratelimit/src/redis.checkError(...)
        /home/moderation/Library/envoyproxy/ratelimit/src/redis/driver_impl.go:44
github.com/envoyproxy/ratelimit/src/redis.NewPoolImpl(0xca07e0, 0xc00010eb40, 0x0, 0x0, 0x0, 0xc00003a2ea, 0xf, 0xa, 0xc00011e080, 0xc000134000)
        /home/moderation/Library/envoyproxy/ratelimit/src/redis/driver_impl.go:98 +0x344
github.com/envoyproxy/ratelimit/src/service_cmd/runner.(*Runner).Run(0xc000213f68)
        /home/moderation/Library/envoyproxy/ratelimit/src/service_cmd/runner/runner.go:57 +0x317
main.main()
        /home/moderation/Library/envoyproxy/ratelimit/src/service_cmd/main.go:7 +0x43

The socket type setting at https://github.com/envoyproxy/ratelimit/blob/master/src/settings/settings.go#L22 is no longer used in the code base.

moderation avatar Apr 12 '20 17:04 moderation

The following unblocks me but I'd suggest we need tests to ensure that default functionality can't be deleted without anyone noticing.

diff --git a/src/redis/driver_impl.go b/src/redis/driver_impl.go
index dbcf787..d50fb56 100644
--- a/src/redis/driver_impl.go
+++ b/src/redis/driver_impl.go
@@ -67,7 +67,7 @@ func (this *poolImpl) Put(c Connection) {
        }
 }

-func NewPoolImpl(scope stats.Scope, useTls bool, auth string, url string, poolSize int) Pool {
+func NewPoolImpl(scope stats.Scope, useTls bool, auth string, url string, poolSize int, socketType string) Pool {
        logger.Warnf("connecting to redis on %s with pool size %d", url, poolSize)
        df := func(network, addr string) (*redis.Client, error) {
                var conn net.Conn
@@ -75,7 +75,7 @@ func NewPoolImpl(scope stats.Scope, useTls bool, auth string, url string, poolSi
                if useTls {
                        conn, err = tls.Dial("tcp", addr, &tls.Config{})
                } else {
-                       conn, err = net.Dial("tcp", addr)
+                       conn, err = net.Dial(socketType, addr)
                }
                if err != nil {
                        return nil, err
diff --git a/src/service_cmd/runner/runner.go b/src/service_cmd/runner/runner.go
index ee9da4b..cd51408 100644
--- a/src/service_cmd/runner/runner.go
+++ b/src/service_cmd/runner/runner.go
@@ -51,10 +51,10 @@ func (runner *Runner) Run() {

        var perSecondPool redis.Pool
        if s.RedisPerSecond {
-               perSecondPool = redis.NewPoolImpl(srv.Scope().Scope("redis_per_second_pool"), s.RedisPerSecondTls, s.RedisPerSecondAuth, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize)
+               perSecondPool = redis.NewPoolImpl(srv.Scope().Scope("redis_per_second_pool"), s.RedisPerSecondTls, s.RedisPerSecondAuth, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize, s.RedisSocketType)
        }
        var otherPool redis.Pool
-       otherPool = redis.NewPoolImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisUrl, s.RedisPoolSize)
+       otherPool = redis.NewPoolImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisUrl, s.RedisPoolSize, s.RedisSocketType)

        service := ratelimit.NewService(
                srv.Runtime(),

moderation avatar Apr 12 '20 17:04 moderation

This issue has been automatically marked as stale because it has not had activity in the last 30 days. It will be closed in the next 7 days unless it is tagged "help wanted" or other activity occurs. Thank you for your contributions.

stale[bot] avatar May 13 '20 03:05 stale[bot]

Not stale. Still broken. Will try to fix next week.

moderation avatar May 13 '20 17:05 moderation