codygateway: Use only one redis pool and make REDIS_ENDPOINT a clear requirement in config (#63625)

Currently, nothing really tells that Cody Gateway needs redis, the env
var for finding the address is hidden somewhere deep in the redispool
package.
In practice, we only use one redis instance, but at some point we
started using both redispool.Cache and redispool.Store, which means we
maintain two connection pools, leading to more than expected
connections.

Test plan:

Code review and CI.

cmd/cody-gateway/internal/httpapi/BUILD.bazel

@@ -36,6 +36,7 @@ go_library(
         "//internal/version",
         "//lib/errors",
         "//lib/pointers",
+        "@com_github_gomodule_redigo//redis",
         "@com_github_gorilla_mux//:mux",
         "@com_github_khan_genqlient//graphql",
         "@com_github_sourcegraph_log//:log",

cmd/cody-gateway/internal/httpapi/diagnostics.go

@@ -6,6 +6,7 @@ import (
 	"net/http"
 	"strings"
 
+	"github.com/gomodule/redigo/redis"
 	"github.com/sourcegraph/log"
 	"github.com/sourcegraph/log/hook"
 	"github.com/sourcegraph/log/output"
@@ -16,7 +17,6 @@ import (
 	"github.com/sourcegraph/sourcegraph/cmd/cody-gateway/internal/response"
 	"github.com/sourcegraph/sourcegraph/internal/authbearer"
 	"github.com/sourcegraph/sourcegraph/internal/instrumentation"
-	"github.com/sourcegraph/sourcegraph/internal/redispool"
 	sgtrace "github.com/sourcegraph/sourcegraph/internal/trace"
 	"github.com/sourcegraph/sourcegraph/internal/version"
 	"github.com/sourcegraph/sourcegraph/lib/errors"
@@ -26,7 +26,7 @@ import (
 // on "/-/..." paths. It should be placed before any authentication middleware, since
 // we do a simple auth on a static secret instead that is uniquely generated per
 // deployment.
-func NewDiagnosticsHandler(baseLogger log.Logger, next http.Handler, secret string, sources *actor.Sources) http.Handler {
+func NewDiagnosticsHandler(baseLogger log.Logger, next http.Handler, redisPool *redis.Pool, secret string, sources *actor.Sources) http.Handler {
 	baseLogger = baseLogger.Scoped("diagnostics")
 
 	hasValidSecret := func(l log.Logger, w http.ResponseWriter, r *http.Request) (yes bool) {
@@ -58,7 +58,7 @@ func NewDiagnosticsHandler(baseLogger log.Logger, next http.Handler, secret stri
 				return
 			}
 
-			if err := healthz(r.Context()); err != nil {
+			if err := healthz(r.Context(), redisPool); err != nil {
 				logger.Error("check failed", log.Error(err))
 
 				w.WriteHeader(http.StatusInternalServerError)
@@ -110,9 +110,8 @@ func NewDiagnosticsHandler(baseLogger log.Logger, next http.Handler, secret stri
 	})
 }
 
-func healthz(ctx context.Context) error {
+func healthz(ctx context.Context, rpool *redis.Pool) error {
 	// Check redis health
-	rpool := redispool.Cache.Pool()
 	rconn, err := rpool.GetContext(ctx)
 	if err != nil {
 		return errors.Wrap(err, "redis: failed to get conn")

cmd/cody-gateway/shared/BUILD.bazel

@@ -5,6 +5,7 @@ go_library(
     srcs = [
         "main.go",
         "metrics.go",
+        "redis.go",
         "service.go",
         "tracing.go",
     ],
@@ -32,6 +33,7 @@ go_library(
         "//internal/goroutine",
         "//internal/httpcli",
         "//internal/httpserver",
+        "//internal/lazyregexp",
         "//internal/observation",
         "//internal/rcache",
         "//internal/redispool",

cmd/cody-gateway/shared/config/config.go

@@ -26,6 +26,8 @@ type Config struct {
 
 	Port int
 
+	RedisEndpoint string
+
 	DiagnosticsSecret string
 
 	Dotcom struct {
@@ -385,6 +387,8 @@ func (c *Config) Load() {
 	c.SAMSClientConfig.ClientSecret = c.GetOptional("SAMS_CLIENT_SECRET", "SAMS OAuth client secret")
 
 	c.Environment = c.Get("CODY_GATEWAY_ENVIRONMENT", "dev", "Environment name.")
+
+	c.RedisEndpoint = c.Get("REDIS_ENDPOINT", "", "Redis endpoint to connect to for storing KV data.")
 }
 
 // loadFlaggingConfig loads the common set of flagging-related environment variables for

cmd/cody-gateway/shared/main.go

@@ -98,6 +98,8 @@ func Main(ctx context.Context, obctx *observation.Context, ready service.ReadyFu
 		}
 	}
 
+	redisPool := connectToRedis(cfg.RedisEndpoint)
+
 	// Create an uncached external doer, we never want to cache any responses.
 	// Not only is the cache hit rate going to be really low and requests large-ish,
 	// but also do we not want to retain any data.
@@ -113,7 +115,7 @@ func Main(ctx context.Context, obctx *observation.Context, ready service.ReadyFu
 		return errors.Wrap(err, "init metric 'redis_latency'")
 	}
 
-	redisCache := redispool.Cache.WithLatencyRecorder(func(call string, latency time.Duration, err error) {
+	redisCache := redispool.RedisKeyValue(redisPool).WithLatencyRecorder(func(call string, latency time.Duration, err error) {
 		redisLatency.Record(context.Background(), latency.Milliseconds(), metric.WithAttributeSet(attribute.NewSet(
 			attribute.Bool("error", err != nil),
 			attribute.String("command", call))))
@@ -238,7 +240,7 @@ func Main(ctx context.Context, obctx *observation.Context, ready service.ReadyFu
 		return errors.Wrap(err, "httpapi.NewHandler")
 	}
 	// Diagnostic and Maintenance layers, exposing additional APIs and endpoints.
-	handler = httpapi.NewDiagnosticsHandler(obctx.Logger, handler, cfg.DiagnosticsSecret, sources)
+	handler = httpapi.NewDiagnosticsHandler(obctx.Logger, handler, redisPool, cfg.DiagnosticsSecret, sources)
 	handler = httpapi.NewMaintenanceHandler(obctx.Logger, handler, cfg, redisCache)
 
 	// Collect request client for downstream handlers. Outside of dev, we always set up
@@ -255,8 +257,7 @@ func Main(ctx context.Context, obctx *observation.Context, ready service.ReadyFu
 	})
 
 	// Set up redis-based distributed mutex for the source syncer worker
-	p := redispool.Store.Pool()
-	sourceWorkerMutex := redsync.New(redigo.NewPool(p)).NewMutex("source-syncer-worker",
+	sourceWorkerMutex := redsync.New(redigo.NewPool(redisPool)).NewMutex("source-syncer-worker",
 		// Do not retry endlessly becuase it's very likely that someone else has
 		// a long-standing hold on the mutex. We will try again on the next periodic
 		// goroutine run.

cmd/cody-gateway/shared/redis.go

@@ -0,0 +1,41 @@
+package shared
+
+import (
+	"strings"
+	"time"
+
+	"github.com/gomodule/redigo/redis"
+
+	"github.com/sourcegraph/sourcegraph/internal/lazyregexp"
+	"github.com/sourcegraph/sourcegraph/lib/errors"
+)
+
+var schemeMatcher = lazyregexp.New(`^[A-Za-z][A-Za-z0-9\+\-\.]*://`)
+
+// connectToRedis connects to Redis given the raw endpoint string.
+// Cody Gateway maintains its own pool of Redis connections, it should not be dependent
+// on the sourcegraph deployment Redis dualism.
+//
+// The string can have two formats:
+//  1. If there is a HTTP scheme, it should be either be "redis://" or "rediss://" and the URL
+//     must be of the format specified in https://www.iana.org/assignments/uri-schemes/prov/redis.
+//  2. Otherwise, it is assumed to be of the format $HOSTNAME:$PORT.
+func connectToRedis(endpoint string) *redis.Pool {
+	return &redis.Pool{
+		MaxIdle:     10,
+		IdleTimeout: 240 * time.Second,
+		TestOnBorrow: func(c redis.Conn, t time.Time) error {
+			_, err := c.Do("PING")
+			return err
+		},
+		Dial: func() (redis.Conn, error) {
+			if schemeMatcher.MatchString(endpoint) { // expect "redis://"
+				return redis.DialURL(endpoint)
+			}
+			if strings.Contains(endpoint, "/") {
+				return nil, errors.New("Redis endpoint without scheme should not contain '/'")
+			}
+			return redis.Dial("tcp", endpoint)
+		},
+	}
+}