redis: fix per-second pipeline config bug and update doc (#240)

Signed-off-by: Tong Cai <[email protected]>
envoyproxy · Mar 30, 2021 · c03723f · c03723f
1 parent 5f6cee9
commit c03723f
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 17 deletions.
diff --git a/README.md b/README.md
@@ -21,7 +21,10 @@
   - [Loading Configuration](#loading-configuration)
   - [Log Format](#log-format)
 - [Request Fields](#request-fields)
+- [GRPC Client](#grpc-client)
+  - [Commandline flags](#commandline-flags)
 - [Statistics](#statistics)
+  - [Statistics options](#statistics-options)
 - [HTTP Port](#http-port)
   - [/json endpoint](#json-endpoint)
 - [Debug Port](#debug-port)
@@ -544,6 +547,8 @@ If window is zero then implicit pipelining will be disabled.
 1. `REDIS_PIPELINE_LIMIT` & `REDIS_PERSECOND_PIPELINE_LIMIT`: sets maximum number of commands that can be pipelined before flushing.
 If limit is zero then no limit will be used and pipelines will only be limited by the specified time window.
 
+`implicit pipelining` is disabled by default. To enable it, you can use default values [used by radix](https://github.com/mediocregopher/radix/blob/v3.5.1/pool.go#L278) and tune for the optimal value.
+
 ## One Redis Instance
 
 To configure one Redis instance use the following environment variables:

diff --git a/src/redis/cache_impl.go b/src/redis/cache_impl.go
@@ -14,7 +14,7 @@ func NewRateLimiterCacheImplFromSettings(s settings.Settings, localCache *freeca
 	var perSecondPool Client
 	if s.RedisPerSecond {
 		perSecondPool = NewClientImpl(srv.Scope().Scope("redis_per_second_pool"), s.RedisPerSecondTls, s.RedisPerSecondAuth,
-			s.RedisPerSecondType, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize, s.RedisPipelineWindow, s.RedisPipelineLimit)
+			s.RedisPerSecondType, s.RedisPerSecondUrl, s.RedisPerSecondPoolSize, s.RedisPerSecondPipelineWindow, s.RedisPerSecondPipelineLimit)
 	}
 	var otherPool Client
 	otherPool = NewClientImpl(srv.Scope().Scope("redis_pool"), s.RedisTls, s.RedisAuth, s.RedisType, s.RedisUrl, s.RedisPoolSize,

diff --git a/src/settings/settings.go b/src/settings/settings.go
@@ -39,23 +39,32 @@ type Settings struct {
 	BackendType                string  `envconfig:"BACKEND_TYPE" default:"redis"`
 
 	// Redis settings
-	RedisSocketType              string        `envconfig:"REDIS_SOCKET_TYPE" default:"unix"`
-	RedisType                    string        `envconfig:"REDIS_TYPE" default:"SINGLE"`
-	RedisUrl                     string        `envconfig:"REDIS_URL" default:"/var/run/nutcracker/ratelimit.sock"`
-	RedisPoolSize                int           `envconfig:"REDIS_POOL_SIZE" default:"10"`
-	RedisAuth                    string        `envconfig:"REDIS_AUTH" default:""`
-	RedisTls                     bool          `envconfig:"REDIS_TLS" default:"false"`
-	RedisPipelineWindow          time.Duration `envconfig:"REDIS_PIPELINE_WINDOW" default:"0"`
-	RedisPipelineLimit           int           `envconfig:"REDIS_PIPELINE_LIMIT" default:"0"`
-	RedisPerSecond               bool          `envconfig:"REDIS_PERSECOND" default:"false"`
-	RedisPerSecondSocketType     string        `envconfig:"REDIS_PERSECOND_SOCKET_TYPE" default:"unix"`
-	RedisPerSecondType           string        `envconfig:"REDIS_PERSECOND_TYPE" default:"SINGLE"`
-	RedisPerSecondUrl            string        `envconfig:"REDIS_PERSECOND_URL" default:"/var/run/nutcracker/ratelimitpersecond.sock"`
-	RedisPerSecondPoolSize       int           `envconfig:"REDIS_PERSECOND_POOL_SIZE" default:"10"`
-	RedisPerSecondAuth           string        `envconfig:"REDIS_PERSECOND_AUTH" default:""`
-	RedisPerSecondTls            bool          `envconfig:"REDIS_PERSECOND_TLS" default:"false"`
+	RedisSocketType string `envconfig:"REDIS_SOCKET_TYPE" default:"unix"`
+	RedisType       string `envconfig:"REDIS_TYPE" default:"SINGLE"`
+	RedisUrl        string `envconfig:"REDIS_URL" default:"/var/run/nutcracker/ratelimit.sock"`
+	RedisPoolSize   int    `envconfig:"REDIS_POOL_SIZE" default:"10"`
+	RedisAuth       string `envconfig:"REDIS_AUTH" default:""`
+	RedisTls        bool   `envconfig:"REDIS_TLS" default:"false"`
+	// RedisPipelineWindow sets the duration after which internal pipelines will be flushed.
+	// If window is zero then implicit pipelining will be disabled. Radix use 150us for the
+	// default value, see https://github.com/mediocregopher/radix/blob/v3.5.1/pool.go#L278.
+	RedisPipelineWindow time.Duration `envconfig:"REDIS_PIPELINE_WINDOW" default:"0"`
+	// RedisPipelineLimit sets maximum number of commands that can be pipelined before flushing.
+	// If limit is zero then no limit will be used and pipelines will only be limited by the specified time window.
+	RedisPipelineLimit       int    `envconfig:"REDIS_PIPELINE_LIMIT" default:"0"`
+	RedisPerSecond           bool   `envconfig:"REDIS_PERSECOND" default:"false"`
+	RedisPerSecondSocketType string `envconfig:"REDIS_PERSECOND_SOCKET_TYPE" default:"unix"`
+	RedisPerSecondType       string `envconfig:"REDIS_PERSECOND_TYPE" default:"SINGLE"`
+	RedisPerSecondUrl        string `envconfig:"REDIS_PERSECOND_URL" default:"/var/run/nutcracker/ratelimitpersecond.sock"`
+	RedisPerSecondPoolSize   int    `envconfig:"REDIS_PERSECOND_POOL_SIZE" default:"10"`
+	RedisPerSecondAuth       string `envconfig:"REDIS_PERSECOND_AUTH" default:""`
+	RedisPerSecondTls        bool   `envconfig:"REDIS_PERSECOND_TLS" default:"false"`
+	// RedisPerSecondPipelineWindow sets the duration after which internal pipelines will be flushed for per second redis.
+	// See comments of RedisPipelineWindow for details.
 	RedisPerSecondPipelineWindow time.Duration `envconfig:"REDIS_PERSECOND_PIPELINE_WINDOW" default:"0"`
-	RedisPerSecondPipelineLimit  int           `envconfig:"REDIS_PERSECOND_PIPELINE_LIMIT" default:"0"`
+	// RedisPerSecondPipelineLimit sets maximum number of commands that can be pipelined before flushing for per second redis.
+	// See comments of RedisPipelineLimit for details.
+	RedisPerSecondPipelineLimit int `envconfig:"REDIS_PERSECOND_PIPELINE_LIMIT" default:"0"`
 
 	// Memcache settings
 	MemcacheHostPort []string `envconfig:"MEMCACHE_HOST_PORT" default:""`