MM-59934: Add Redis to CI and other improvements (#28164)

- Update library version.
- Added MaxFlush delay to help reduce CPU usage.
- Fall back to LRU cache for the caches which use SCAN.
- Added mattermost-redis and running for all api layer
tests in Postgres.

https://mattermost.atlassian.net/browse/MM-59934
```release-note
NONE
```

server/build/docker-compose-generator/main.go

@@ -32,6 +32,7 @@ func main() {
 		"openldap":           389,
 		"elasticsearch":      9200,
 		"opensearch":         9201,
+		"redis":              6379,
 		"dejavu":             1358,
 		"keycloak":           8080,
 		"prometheus":         9090,

server/build/docker-compose.common.yml

@@ -118,6 +118,10 @@ services:
       discovery.type: single-node
       plugins.security.disabled: "true"
       ES_JAVA_OPTS: "-Xms512m -Xmx512m"
+  redis:
+    image: "redis:7.4.0"
+    networks:
+      - mm-test
   dejavu:
     image: "appbaseio/dejavu:3.4.2"
     networks:

server/build/docker-compose.yml

@@ -32,6 +32,10 @@ services:
     extends:
         file: docker-compose.common.yml
         service: opensearch
+  redis:
+    extends:
+        file: docker-compose.common.yml
+        service: redis
   dejavu:
     extends:
         file: docker-compose.common.yml
@@ -69,7 +73,8 @@ services:
       - openldap
       - elasticsearch
       - opensearch
-    command: postgres:5432 mysql:3306 minio:9000 inbucket:9001 openldap:389 elasticsearch:9200 opensearch:9201
+      - redis
+    command: postgres:5432 mysql:3306 minio:9000 inbucket:9001 openldap:389 elasticsearch:9200 opensearch:9201 redis:6379
 
 networks:
   mm-test:

server/channels/api4/apitestlib.go

@@ -102,6 +102,17 @@ func setupTestHelper(dbStore store.Store, searchEngine *searchengine.Broker, ent
 	*memoryConfig.AnnouncementSettings.AdminNoticesEnabled = false
 	*memoryConfig.AnnouncementSettings.UserNoticesEnabled = false
 	*memoryConfig.PluginSettings.AutomaticPrepackagedPlugins = false
+	// Enabling Redis with Postgres.
+	if *memoryConfig.SqlSettings.DriverName == model.DatabaseDriverPostgres {
+		*memoryConfig.CacheSettings.CacheType = model.CacheTypeRedis
+		redisHost := "localhost"
+		if os.Getenv("IS_CI") == "true" {
+			redisHost = "redis"
+		}
+		*memoryConfig.CacheSettings.RedisAddress = redisHost + ":6379"
+		*memoryConfig.CacheSettings.DisableClientCache = true
+		*memoryConfig.CacheSettings.RedisDB = 0
+	}
 	if updateConfig != nil {
 		updateConfig(memoryConfig)
 	}

server/channels/api4/config_test.go

@@ -515,7 +515,7 @@ func TestUpdateConfigDiffInAuditRecord(t *testing.T) {
 	require.NotEmpty(t, data)
 
 	require.Contains(t, string(data),
-		fmt.Sprintf(`"config_diffs":[{"actual_val":%d,"base_val":%d,"path":"ServiceSettings.ReadTimeout"}]`,
+		fmt.Sprintf(`"config_diffs":[{"actual_val":%d,"base_val":%d,"path":"ServiceSettings.ReadTimeout"}`,
 			timeoutVal+1, timeoutVal))
 }
 

server/channels/api4/metrics_test.go

@@ -30,6 +30,7 @@ func setupMetricsMock() *mocks.MetricsInterface {
 	metricsMock.On("IncrementHTTPError").Return()
 	metricsMock.On("IncrementHTTPRequest").Return()
 	metricsMock.On("ObserveAPIEndpointDuration", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return()
+	metricsMock.On("ObserveRedisEndpointDuration", mock.AnythingOfType("string"), mock.AnythingOfType("string"), mock.AnythingOfType("float64")).Return()
 	metricsMock.On("Register").Return()
 
 	return metricsMock

server/channels/app/platform/service.go

@@ -169,6 +169,7 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) {
 				RedisAddr:     *cacheConfig.RedisAddress,
 				RedisPassword: *cacheConfig.RedisPassword,
 				RedisDB:       *cacheConfig.RedisDB,
+				DisableCache:  *cacheConfig.DisableClientCache,
 			},
 		)
 	}
@@ -292,8 +293,13 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) {
 		return nil, fmt.Errorf("cannot create store: %w", err)
 	}
 
+	// Note: we hardcode the session and status cache to LRU because they lead
+	// to a lot of SCAN calls in case of Redis. We could potentially have a
+	// reverse mapping to avoid the scan, but this needs more complicated code.
+	// Leaving this for now.
+
 	// Needed before loading license
-	ps.statusCache, err = ps.cacheProvider.NewCache(&cache.CacheOptions{
+	ps.statusCache, err = cache.NewProvider().NewCache(&cache.CacheOptions{
 		Name:           "Status",
 		Size:           model.StatusCacheSize,
 		Striped:        true,
@@ -304,10 +310,6 @@ func New(sc ServiceConfig, options ...Option) (*PlatformService, error) {
 		return nil, fmt.Errorf("unable to create status cache: %w", err)
 	}
 
-	// Note: we hardcode the session cache to LRU because the session invalidation
-	// path always iterates through the entire cache, leading to a lot of SCAN calls
-	// in case of Redis. We could potentially have a reverse mapping of userIDs to
-	// session IDs, but leaving this one for now.
 	ps.sessionCache, err = cache.NewProvider().NewCache(&cache.CacheOptions{
 		Name:           "Session",
 		Size:           model.SessionCacheSize,

server/channels/store/localcachelayer/layer.go

@@ -340,7 +340,8 @@ func NewLocalCacheLayer(baseStore store.Store, metrics einterfaces.MetricsInterf
 	}); err != nil {
 		return
 	}
-	if localCacheStore.profilesInChannelCache, err = cacheProvider.NewCache(&cache.CacheOptions{
+	// Hardcoding this to LRU because of the volume of SCAN calls in case of Redis.
+	if localCacheStore.profilesInChannelCache, err = cache.NewProvider().NewCache(&cache.CacheOptions{
 		Size:                   ProfilesInChannelCacheSize,
 		Name:                   "ProfilesInChannel",
 		DefaultExpiry:          ProfilesInChannelCacheSec * time.Second,
@@ -526,7 +527,10 @@ func (s *LocalCacheStore) doMultiReadCache(cache cache.Cache, keys []string, val
 }
 
 func (s *LocalCacheStore) doClearCacheCluster(cache cache.Cache) {
-	cache.Purge()
+	err := cache.Purge()
+	if err != nil {
+		s.logger.Warn("Error while purging cache", mlog.Err(err), mlog.String("cache_name", cache.Name()))
+	}
 	if s.cluster != nil && s.cacheType == model.CacheTypeLRU {
 		msg := &model.ClusterMessage{
 			Event:    cache.GetInvalidateClusterEvent(),

server/config.mk

@@ -5,12 +5,13 @@
 # Enable services to be run in docker.
 #
 # Possible options: mysql, postgres, minio, inbucket, openldap, dejavu,
-# keycloak, elasticsearch, prometheus, grafana, loki and promtail.
+# keycloak, elasticsearch, opensearch, redis, prometheus,
+# grafana, loki and promtail.
 #
 # Must be space separated names.
 #
 # Example: mysql postgres elasticsearch
-ENABLED_DOCKER_SERVICES ?= mysql postgres inbucket
+ENABLED_DOCKER_SERVICES ?= mysql postgres inbucket redis
 
 # Disable entirely the use of docker
 MM_NO_DOCKER ?= false

server/docker-compose.makefile.yml

@@ -67,6 +67,13 @@ services:
     extends:
         file: build/docker-compose.common.yml
         service: opensearch
+  redis:
+    container_name: mattermost-redis
+    ports:
+      - "6379:6379"
+    extends:
+        file: build/docker-compose.common.yml
+        service: redis
   dejavu:
     restart: 'no'
     container_name: mattermost-dejavu

server/docker-compose.yaml

@@ -60,6 +60,13 @@ services:
     extends:
         file: build/docker-compose.common.yml
         service: opensearch
+  redis:
+    container_name: mattermost-redis
+    ports:
+      - "6379:6379"
+    extends:
+        file: build/docker-compose.common.yml
+        service: redis
   dejavu:
     container_name: mattermost-dejavu
     ports:

server/go.mod

@@ -55,7 +55,7 @@ require (
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/client_golang v1.19.1
 	github.com/prometheus/client_model v0.6.1
-	github.com/redis/rueidis v1.0.41
+	github.com/redis/rueidis v1.0.45
 	github.com/reflog/dateconstraints v0.2.1
 	github.com/rs/cors v1.11.0
 	github.com/rudderlabs/analytics-go v3.3.3+incompatible
@@ -225,7 +225,7 @@ require (
 	go.uber.org/multierr v1.11.0 // indirect
 	golang.org/x/exp v0.0.0-20240719175910-8a7402abbf56 // indirect
 	golang.org/x/mod v0.19.0 // indirect
-	golang.org/x/sys v0.22.0 // indirect
+	golang.org/x/sys v0.24.0 // indirect
 	golang.org/x/text v0.16.0 // indirect
 	google.golang.org/genproto/googleapis/rpc v0.0.0-20240722135656-d784300faade // indirect
 	google.golang.org/grpc v1.65.0 // indirect

server/go.sum

@@ -436,8 +436,8 @@ github.com/oklog/run v1.1.0/go.mod h1:sVPdnTZT1zYwAJeCMu2Th4T21pA3FPOQRfWjQlk7DV
 github.com/olekukonko/tablewriter v0.0.0-20180506121414-d4647c9c7a84/go.mod h1:vsDQFd/mU46D+Z4whnwzcISnGGzXWMclvtLoiIKAKIo=
 github.com/olekukonko/tablewriter v0.0.5 h1:P2Ga83D34wi1o9J6Wh1mRuqd4mF/x/lgBS7N7AbDhec=
 github.com/olekukonko/tablewriter v0.0.5/go.mod h1:hPp6KlRPjbx+hW8ykQs1w3UBbZlj6HuIJcUGPhkA7kY=
-github.com/onsi/gomega v1.31.1 h1:KYppCUK+bUgAZwHOu7EXVBKyQA6ILvOESHkn/tgoqvo=
-github.com/onsi/gomega v1.31.1/go.mod h1:y40C95dwAD1Nz36SsEnxvfFe8FFfNxzI5eJ0EYGyAy0=
+github.com/onsi/gomega v1.34.1 h1:EUMJIKUjM8sKjYbtxQI9A4z2o+rruxnzNvpknOXie6k=
+github.com/onsi/gomega v1.34.1/go.mod h1:kU1QgUvBDLXBJq618Xvm2LUX6rSAfRaFRTcdOeDLwwY=
 github.com/oov/psd v0.0.0-20220121172623-5db5eafcecbb h1:JF9kOhBBk4WPF7luXFu5yR+WgaFm9L/KiHJHhU9vDwA=
 github.com/oov/psd v0.0.0-20220121172623-5db5eafcecbb/go.mod h1:GHI1bnmAcbp96z6LNfBJvtrjxhaXGkbsk967utPlvL8=
 github.com/opentracing/opentracing-go v1.2.0 h1:uEJPy/1a5RIPAJ0Ov+OIO8OxWu77jEv+1B0VhjKrZUs=
@@ -495,8 +495,8 @@ github.com/prometheus/procfs v0.15.1 h1:YagwOFzUgYfKKHX6Dr+sHT7km/hxC76UB0leargg
 github.com/prometheus/procfs v0.15.1/go.mod h1:fB45yRUv8NstnjriLhBQLuOUt+WW4BsoGhij/e3PBqk=
 github.com/redis/go-redis/v9 v9.6.0 h1:NLck+Rab3AOTHw21CGRpvQpgTrAU4sgdCswqGtlhGRA=
 github.com/redis/go-redis/v9 v9.6.0/go.mod h1:hdY0cQFCN4fnSYT6TkisLufl/4W5UIXyv0b/CLO2V2M=
-github.com/redis/rueidis v1.0.41 h1:Ls5Dto11Tknh8CFbaMTSBD0fgMlTqJWq8/Df4LPaeQ0=
-github.com/redis/rueidis v1.0.41/go.mod h1:bnbkk4+CkXZgDPEbUtSos/o55i4RhFYYesJ4DS2zmq0=
+github.com/redis/rueidis v1.0.45 h1:j7hfcqfLLIqgTK3IkxBhXdeJcP34t3XLXvorDLqXfgM=
+github.com/redis/rueidis v1.0.45/go.mod h1:by+34b0cFXndxtYmPAHpoTHO5NkosDlBvhexoTURIxM=
 github.com/reflog/dateconstraints v0.2.1 h1:Hz1n2Q1vEm0Rj5gciDQcCN1iPBwfFjxUJy32NknGP/s=
 github.com/reflog/dateconstraints v0.2.1/go.mod h1:Ax8AxTBcJc3E/oVS2hd2j7RDM/5MDtuPwuR7lIHtPLo=
 github.com/remyoudompheng/bigfft v0.0.0-20230129092748-24d4a6f8daec h1:W09IVJc94icq4NjY3clb7Lk8O1qJ8BdBEF8z0ibU0rE=
@@ -775,8 +775,8 @@ golang.org/x/sys v0.8.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.13.0/go.mod h1:oPkhp1MJrh7nUepCBck5+mAzfO9JrbApNNgaTdGDITg=
 golang.org/x/sys v0.17.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/sys v0.20.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
-golang.org/x/sys v0.22.0 h1:RI27ohtqKCnwULzJLqkv897zojh5/DwS/ENaMzUOaWI=
-golang.org/x/sys v0.22.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
+golang.org/x/sys v0.24.0 h1:Twjiwq9dn6R1fQcyiK+wQyHWfaz/BJB+YIpzU/Cv3Xg=
+golang.org/x/sys v0.24.0/go.mod h1:/VUhepiaJMQUp4+oa/7Zr1D23ma6VTLIYjOOTFZPUcA=
 golang.org/x/term v0.0.0-20201126162022-7de9c90e9dd1/go.mod h1:bj7SfCRtBDWHUb9snDiAeCFNEtKQo2Wmx5Cou7ajbmo=
 golang.org/x/term v0.0.0-20210927222741-03fcf44c2211/go.mod h1:jbD1KX2456YbFQfuXm/mYQcufACuNUgVhRMnK/tPxf8=
 golang.org/x/term v0.5.0/go.mod h1:jMB1sMXY+tzblOD4FWmEbocvup2/aLOaQEp7JmGp78k=

server/platform/services/cache/provider.go

@@ -82,6 +82,7 @@ type RedisOptions struct {
 	RedisAddr     string
 	RedisPassword string
 	RedisDB       int
+	DisableCache  bool
 }
 
 // NewProvider creates a new CacheProvider
@@ -91,8 +92,14 @@ func NewRedisProvider(opts *RedisOptions) (Provider, error) {
 		Password:          opts.RedisPassword,
 		SelectDB:          opts.RedisDB,
 		ForceSingleClient: true,
-		CacheSizeEachConn: 16 * (1 << 20), // 16MiB local cache size
-		//TODO: look into MaxFlushDelay
+		CacheSizeEachConn: 32 * (1 << 20), // 32MiB local cache size
+		DisableCache:      opts.DisableCache,
+		// This is used to collect more commands before flushing to Redis.
+		// This increases latency at the cost of lower CPU usage at Redis.
+		// It's a tradeoff we are willing to make because Redis is only
+		// meant to be used at very high scales. The docs suggest 20us,
+		// but going as high as 250us doesn't make any material difference.
+		MaxFlushDelay: 250 * time.Microsecond,
 	})
 	if err != nil {
 		return nil, err

server/public/model/config.go

@@ -935,27 +935,32 @@ func (s *ServiceSettings) SetDefaults(isUpdate bool) {
 }
 
 type CacheSettings struct {
-	CacheType     *string `access:",write_restrictable,cloud_restrictable"`
-	RedisAddress  *string `access:",write_restrictable,cloud_restrictable"` // telemetry: none
-	RedisPassword *string `access:",write_restrictable,cloud_restrictable"` // telemetry: none
-	RedisDB       *int    `access:",write_restrictable,cloud_restrictable"` // telemetry: none
+	CacheType          *string `access:",write_restrictable,cloud_restrictable"`
+	RedisAddress       *string `access:",write_restrictable,cloud_restrictable"` // telemetry: none
+	RedisPassword      *string `access:",write_restrictable,cloud_restrictable"` // telemetry: none
+	RedisDB            *int    `access:",write_restrictable,cloud_restrictable"` // telemetry: none
+	DisableClientCache *bool   `access:",write_restrictable,cloud_restrictable"` // telemetry: none
 }
 
 func (s *CacheSettings) SetDefaults() {
 	if s.CacheType == nil {
-		s.CacheType = NewString(CacheTypeLRU)
+		s.CacheType = NewPointer(CacheTypeLRU)
 	}
 
 	if s.RedisAddress == nil {
-		s.RedisAddress = NewString("")
+		s.RedisAddress = NewPointer("")
 	}
 
 	if s.RedisPassword == nil {
-		s.RedisPassword = NewString("")
+		s.RedisPassword = NewPointer("")
 	}
 
 	if s.RedisDB == nil {
-		s.RedisDB = NewInt(-1)
+		s.RedisDB = NewPointer(-1)
+	}
+
+	if s.DisableClientCache == nil {
+		s.DisableClientCache = NewPointer(false)
 	}
 }
 
@@ -4507,6 +4512,10 @@ func (o *Config) Sanitize(pluginManifests []*Manifest) {
 		*o.ServiceSettings.SplitKey = FakeSetting
 	}
 
+	if o.CacheSettings.RedisPassword != nil {
+		*o.CacheSettings.RedisPassword = FakeSetting
+	}
+
 	o.PluginSettings.Sanitize(pluginManifests)
 }