redis: support REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR

README.md

@@ -1388,15 +1388,13 @@ Rate limit calls carry the deadline set by the client (e.g. Envoy's rate limit f
 
 ### Pool On-Empty Behavior
 
-Controls what happens when all connections in the pool are in use and a new request arrives.
-
-1. `REDIS_POOL_ON_EMPTY_BEHAVIOR`: controls what happens when the pool is empty. Default: `CREATE`
-   - `CREATE`: create a new overflow connection after waiting for `REDIS_POOL_ON_EMPTY_WAIT_DURATION`. This is the [default radix behavior](https://github.com/mediocregopher/radix/blob/v3.8.1/pool.go#L291-L312).
-   - `ERROR`: return an error after waiting for `REDIS_POOL_ON_EMPTY_WAIT_DURATION`. This enforces a strict pool size limit.
-   - `WAIT`: block until a connection becomes available. This enforces a strict pool size limit but may cause goroutine buildup.
-1. `REDIS_POOL_ON_EMPTY_WAIT_DURATION`: the duration to wait before taking the configured action (`CREATE` or `ERROR`). Default: `1s`
-1. `REDIS_PERSECOND_POOL_ON_EMPTY_BEHAVIOR`: same as above for per-second Redis pool. Default: `CREATE`
-1. `REDIS_PERSECOND_POOL_ON_EMPTY_WAIT_DURATION`: same as above for per-second Redis pool. Default: `1s`
+The pool keeps a fixed number of connections, and rate limit calls share them, so a call only has to wait for a connection when the pool has none: Redis is unreachable and the pool is re-dialing.
+
+1. `REDIS_POOL_ON_EMPTY_BEHAVIOR`: controls what happens when a call needs a connection and the pool has none. Default: `WAIT`
+   - `WAIT`: wait for a connection, up to the call's deadline.
+   - `ERROR`: fail the call right away.
+   - `CREATE`: not supported; ratelimit refuses to start. The pool cannot create overflow connections, consider increasing `REDIS_POOL_SIZE` instead.
+1. `REDIS_PERSECOND_POOL_ON_EMPTY_BEHAVIOR`: same as above for per-second Redis pool. Default: `WAIT`
 
 ### Pipelining
 

src/redis/deadline.go

@@ -7,6 +7,10 @@ import (
 	"reflect"
 )
 
+// ErrNoConnection is returned, with REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR, when
+// the pool has no connection to Redis.
+var ErrNoConnection = errors.New("no Redis connection available")
+
 // ErrTooManyInflight is returned when a Redis client already has the maximum
 // number of commands in flight (REDIS_MAX_INFLIGHT).
 var ErrTooManyInflight = errors.New("too many Redis commands in flight")
@@ -30,6 +34,10 @@ func (c *clientImpl) do(ctx context.Context, fn func(context.Context) error, onS
 		return fmt.Errorf("not sending Redis command: %w", err)
 	}
 
+	if c.failWhenNoConnection && c.stats.connectionActive.Value() == 0 {
+		return ErrNoConnection
+	}
+
 	if c.inflight != nil {
 		select {
 		case c.inflight <- struct{}{}:

src/redis/deadline_test.go

@@ -7,6 +7,7 @@ import (
 	"testing"
 	"time"
 
+	stats "github.com/lyft/gostats"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 )
@@ -149,3 +150,29 @@ func TestCopyReply(t *testing.T) {
 }
 
 func newUint64(v uint64) *uint64 { return &v }
+
+func TestPoolOnEmptyErrorFailsFastWithoutConnections(t *testing.T) {
+	srv := newFakeRedis(t, nil)
+	store := stats.NewStore(stats.NewNullSink(), false)
+	c := newClientImpl(context.Background(), store, false, "", "tcp", "single", srv.addr(), 2,
+		0, 0, nil, false, nil, time.Second, "ERROR", "", 10*time.Millisecond, 100*time.Millisecond,
+		time.Second, 1, false, nil, clientOptions{ioTimeout: 100 * time.Millisecond}).(*clientImpl)
+	defer c.Close()
+
+	var rcv uint64
+	require.NoError(t, c.PipeDo(context.Background(), incrPipeline(c, &rcv)))
+
+	// Redis goes away: once the pool has dropped its connections, commands
+	// fail right away instead of waiting for a connection.
+	srv.close()
+	require.Eventually(t, func() bool {
+		var rcv uint64
+		_ = c.PipeDo(context.Background(), incrPipeline(c, &rcv))
+		return c.NumActiveConns() == 0
+	}, 5*time.Second, 10*time.Millisecond)
+
+	start := time.Now()
+	err := c.PipeDo(context.Background(), incrPipeline(c, &rcv))
+	assert.ErrorIs(t, err, ErrNoConnection)
+	assert.Less(t, time.Since(start), 10*time.Millisecond)
+}

src/redis/driver_impl.go

@@ -82,6 +82,9 @@ type clientImpl struct {
 	clusterPipelineParallelism int
 	// inflight bounds the number of commands in flight, nil if unbounded.
 	inflight chan struct{}
+	// failWhenNoConnection makes commands fail right away while the pool has
+	// no connection (REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR).
+	failWhenNoConnection bool
 }
 
 func checkError(err error) {
@@ -258,37 +261,25 @@ func newClientImpl(ctx context.Context, scope stats.Scope, useTls bool, auth, re
 		}
 	}
 
-	// IMPORTANT: radix v4 pool behavior changes from v3
+	// radix v4 pools have a fixed size, and pipelines share connections, so a
+	// command only waits for a connection when the pool has none, i.e. when
+	// Redis is unreachable and the pool is re-dialing. That wait is bounded by
+	// the request's context (see clientImpl.do).
 	//
-	// v4 uses a FIXED pool size and BLOCKS when all connections are in use.
-	// This is the same as v3's WAIT behavior.
-	//
-	// v3 CREATE and ERROR behaviors are NOT supported in v4:
-	// - v3 WAIT   → v4 supported (blocks until connection available)
-	// - v3 CREATE → v4 NOT SUPPORTED (would block instead of creating overflow connections)
-	// - v3 ERROR  → v4 NOT SUPPORTED (would block instead of failing fast)
-	//
-	// Migration requirements:
-	// - Remove REDIS_POOL_ON_EMPTY_BEHAVIOR setting if set to CREATE or ERROR
-	// - Use WAIT or leave unset (WAIT is default)
-	// - Consider increasing REDIS_POOL_SIZE if you previously relied on CREATE
-	// - Use context timeouts to prevent indefinite blocking:
-	//     ctx, cancel := context.WithTimeout(context.Background(), 1*time.Second)
-	//     defer cancel()
-	//     client.Do(ctx, cmd)
+	// - WAIT (default): wait for a connection, up to the request deadline.
+	// - ERROR: fail right away when the pool has no connection.
+	// - CREATE: not supported; radix v4 cannot create overflow connections.
+	failWhenNoConnection := false
 	switch strings.ToUpper(poolOnEmptyBehavior) {
 	case "WAIT":
-		logger.Warnf("Redis pool %s: WAIT is default in radix v4 (blocks until connection available)", maskedUrl)
+		logger.Warnf("Redis pool %s: on-empty=WAIT (commands wait for a connection up to their deadline)", maskedUrl)
 	case "CREATE":
-		// v3 CREATE created overflow connections when pool was full
-		// v4 does NOT support this - fail fast to prevent unexpected blocking behavior
-		panic(RedisError("REDIS_POOL_ON_EMPTY_BEHAVIOR=CREATE is not supported in radix v4. Pool will block instead of creating overflow connections. Remove this setting or set to WAIT, and consider increasing REDIS_POOL_SIZE."))
+		panic(RedisError("REDIS_POOL_ON_EMPTY_BEHAVIOR=CREATE is not supported in radix v4: the pool has a fixed size and cannot create overflow connections. Remove this setting or set it to WAIT or ERROR, and consider increasing REDIS_POOL_SIZE."))
 	case "ERROR":
-		// v3 ERROR failed fast when pool was full
-		// v4 does NOT support this - fail fast to prevent unexpected blocking behavior
-		panic(RedisError("REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR is not supported in radix v4. Pool will block instead of failing fast. Remove this setting or set to WAIT, and use context timeouts for fail-fast behavior."))
+		logger.Warnf("Redis pool %s: on-empty=ERROR (commands fail right away while the pool has no connection)", maskedUrl)
+		failWhenNoConnection = true
 	default:
-		logger.Warnf("Redis pool %s: using v4 default (fixed size=%d, blocks when full)", maskedUrl, poolSize)
+		logger.Warnf("Redis pool %s: using v4 default (fixed size=%d, commands wait for a connection up to their deadline)", maskedUrl, poolSize)
 	}
 
 	poolFunc := func(ctx context.Context, network, addr string) (radix.Client, error) {
@@ -384,6 +375,7 @@ func newClientImpl(ctx context.Context, scope stats.Scope, useTls bool, auth, re
 		stats:                      stats,
 		isCluster:                  isCluster,
 		clusterPipelineParallelism: effectivePipelineParallelism,
+		failWhenNoConnection:       failWhenNoConnection,
 	}
 	if opts.maxInflight > 0 {
 		logger.Warnf("Redis pool %s: at most %d commands in flight", maskedUrl, opts.maxInflight)

src/settings/settings.go

@@ -265,13 +265,12 @@ type Settings struct {
 	// RedisPerSecondTcpUserTimeout is RedisTcpUserTimeout for the per-second Redis.
 	RedisPerSecondTcpUserTimeout time.Duration `envconfig:"REDIS_PERSECOND_TCP_USER_TIMEOUT" default:"0"`
 
-	// RedisPoolOnEmptyBehavior controls what happens when Redis connection pool is empty.
-	// NOTE: In radix v4, the pool ALWAYS blocks when empty (WAIT behavior).
-	// Possible values:
-	//   - "WAIT": Block until a connection is available (default, radix v4 behavior)
+	// RedisPoolOnEmptyBehavior controls what happens when a command needs a
+	// connection and the pool has none (Redis is unreachable and the pool is
+	// re-dialing). Possible values:
+	//   - "WAIT": wait for a connection, up to the request deadline (default)
+	//   - "ERROR": fail right away
 	//   - "CREATE": NOT SUPPORTED in radix v4 - will cause panic at startup
-	//   - "ERROR": NOT SUPPORTED in radix v4 - will cause panic at startup
-	// For fail-fast behavior, use context timeouts when calling Redis operations.
 	RedisPoolOnEmptyBehavior string `envconfig:"REDIS_POOL_ON_EMPTY_BEHAVIOR" default:"WAIT"`
 
 	// RedisPerSecondPoolOnEmptyBehavior controls pool-empty behavior for per-second Redis.

test/redis/driver_impl_test.go

@@ -274,15 +274,21 @@ func TestPoolOnEmptyBehavior(t *testing.T) {
 		assert.Equal(t, "bar", res)
 	})
 
-	t.Run("ERROR behavior should panic", func(t *testing.T) {
+	t.Run("ERROR behavior", func(t *testing.T) {
 		redisSrv := mustNewRedisServer()
 		defer redisSrv.Close()
 
-		// radix v4 does not support ERROR behavior - should panic at startup
-		panicErr := expectPanicError(t, func() {
-			mkRedisClientWithBehavior(redisSrv.Addr(), "ERROR")
+		var client redis.Client
+		assert.NotPanics(t, func() {
+			client = mkRedisClientWithBehavior(redisSrv.Addr(), "ERROR")
 		})
-		assert.Contains(t, panicErr.Error(), "REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR is not supported in radix v4")
+		assert.NotNil(t, client)
+
+		// Verify client works while connected
+		var res string
+		assert.Nil(t, client.DoCmd(nil, "SET", "test4", "value4"))
+		assert.Nil(t, client.DoCmd(&res, "GET", "test4"))
+		assert.Equal(t, "value4", res)
 	})
 
 	t.Run("CREATE behavior should panic", func(t *testing.T) {
@@ -313,15 +319,13 @@ func TestPoolOnEmptyBehavior(t *testing.T) {
 		assert.Equal(t, "value5", res)
 	})
 
-	t.Run("case insensitive behavior - lowercase 'error' panics", func(t *testing.T) {
+	t.Run("case insensitive behavior - lowercase 'error' works", func(t *testing.T) {
 		redisSrv := mustNewRedisServer()
 		defer redisSrv.Close()
 
-		// Test that lowercase 'error' is treated same as 'ERROR' (case insensitive)
-		panicErr := expectPanicError(t, func() {
+		assert.NotPanics(t, func() {
 			mkRedisClientWithBehavior(redisSrv.Addr(), "error")
 		})
-		assert.Contains(t, panicErr.Error(), "REDIS_POOL_ON_EMPTY_BEHAVIOR=ERROR is not supported in radix v4")
 	})
 
 	t.Run("case insensitive behavior - lowercase 'create' panics", func(t *testing.T) {