CBG-4134: link rev cache memory limit config option to rev cache (#7084)

* CBG-4134: link rev cache memory limit config option to rev cache * failing tests * address commnets * fix yaml lint * fix failing test + mistake in docs Signed-off-by: Gregory Newman-Smith <[email protected]> --------- Signed-off-by: Gregory Newman-Smith <[email protected]>
couchbase · Sep 6, 2024 · 2f5dcd6 · 2f5dcd6
1 parent 1db2b79
commit 2f5dcd6
Show file tree

Hide file tree

Showing 9 changed files with 79 additions and 23 deletions.
diff --git a/docs/api/components/schemas.yaml b/docs/api/components/schemas.yaml
@@ -1304,6 +1304,13 @@ Database:
               description: The maximum number of revisions that can be stored in the revision cache.
               type: string
               default: 5000
+            max_memory_count_mb:
+              description: |-
+                The maximum amount of memory the revision cache should take up in MB, setting to 0 will disable any eviction based on memory at rev cache.
+                When set this memory limit will work in in hand with revision cache size parameter. So you will potentially get eviction at revision cache both based off memory footprint and number of items in the cache.
+                **This is an enterprise-edition feature only**
+              type: integer
+              default: 0
             shard_count:
               description: The number of shards the revision cache should be split into.
               type: string

diff --git a/rest/access_test.go b/rest/access_test.go
@@ -285,7 +285,7 @@ func TestUserHasDocAccessDocNotFound(t *testing.T) {
 			QueryPaginationLimit: base.IntPtr(2),
 			CacheConfig: &CacheConfig{
 				RevCacheConfig: &RevCacheConfig{
-					Size: base.Uint32Ptr(0),
+					MaxItemCount: base.Uint32Ptr(0),
 				},
 				ChannelCacheConfig: &ChannelCacheConfig{
 					MaxNumber: base.IntPtr(0),

diff --git a/rest/adminapitest/admin_api_test.go b/rest/adminapitest/admin_api_test.go
@@ -2547,7 +2547,7 @@ func TestHandleDBConfig(t *testing.T) {
 	dbConfig := rt.NewDbConfig()
 	dbConfig.CacheConfig = &rest.CacheConfig{
 		RevCacheConfig: &rest.RevCacheConfig{
-			Size: base.Uint32Ptr(1337), ShardCount: base.Uint16Ptr(7),
+			MaxItemCount: base.Uint32Ptr(1337), ShardCount: base.Uint16Ptr(7),
 		},
 	}
 

diff --git a/rest/bootstrap_test.go b/rest/bootstrap_test.go
@@ -69,7 +69,7 @@ func TestBootstrapRESTAPISetup(t *testing.T) {
 	assert.Empty(t, dbConfigResp.Username)
 	assert.Empty(t, dbConfigResp.Password)
 	require.Nil(t, dbConfigResp.Sync)
-	require.Equal(t, uint32(1234), *dbConfigResp.CacheConfig.RevCacheConfig.Size)
+	require.Equal(t, uint32(1234), *dbConfigResp.CacheConfig.RevCacheConfig.MaxItemCount)
 
 	// Sanity check to use the database
 	resp = BootstrapAdminRequest(t, sc, http.MethodPut, "/db1/doc1", `{"foo":"bar"}`)
@@ -103,7 +103,7 @@ func TestBootstrapRESTAPISetup(t *testing.T) {
 	assert.Empty(t, dbConfigResp.Username)
 	assert.Empty(t, dbConfigResp.Password)
 	require.Nil(t, dbConfigResp.Sync)
-	require.Equal(t, uint32(1234), *dbConfigResp.CacheConfig.RevCacheConfig.Size)
+	require.Equal(t, uint32(1234), *dbConfigResp.CacheConfig.RevCacheConfig.MaxItemCount)
 
 	// Ensure it's _actually_ the same bucket
 	resp = BootstrapAdminRequest(t, sc, http.MethodGet, "/db1/doc1", ``)

diff --git a/rest/config.go b/rest/config.go
@@ -246,8 +246,9 @@ type DeprecatedCacheConfig struct {
 }
 
 type RevCacheConfig struct {
-	Size       *uint32 `json:"size,omitempty"`        // Maximum number of revisions to store in the revision cache
-	ShardCount *uint16 `json:"shard_count,omitempty"` // Number of shards the rev cache should be split into
+	MaxItemCount     *uint32 `json:"size,omitempty"`                // Maximum number of revisions to store in the revision cache
+	MaxMemoryCountMB *uint32 `json:"max_memory_count_mb,omitempty"` // Maximum amount of memory the rev cache should consume in MB, when configured it will work in tandem with max items
+	ShardCount       *uint16 `json:"shard_count,omitempty"`         // Number of shards the rev cache should be split into
 }
 
 type ChannelCacheConfig struct {
@@ -814,10 +815,15 @@ func (dbConfig *DbConfig) validateVersion(ctx context.Context, isEnterpriseEditi
 
 		if dbConfig.CacheConfig.RevCacheConfig != nil {
 			// EE: disable revcache
-			revCacheSize := dbConfig.CacheConfig.RevCacheConfig.Size
+			revCacheSize := dbConfig.CacheConfig.RevCacheConfig.MaxItemCount
 			if !isEnterpriseEdition && revCacheSize != nil && *revCacheSize == 0 {
 				base.WarnfCtx(ctx, eeOnlyWarningMsg, "cache.rev_cache.size", *revCacheSize, db.DefaultRevisionCacheSize)
-				dbConfig.CacheConfig.RevCacheConfig.Size = nil
+				dbConfig.CacheConfig.RevCacheConfig.MaxItemCount = nil
+			}
+			revCacheMemoryLimit := dbConfig.CacheConfig.RevCacheConfig.MaxMemoryCountMB
+			if !isEnterpriseEdition && revCacheMemoryLimit != nil && *revCacheMemoryLimit != 0 {
+				base.WarnfCtx(ctx, eeOnlyWarningMsg, "cache.rev_cache.max_memory_count_mb", *revCacheMemoryLimit, "no memory limit")
+				dbConfig.CacheConfig.RevCacheConfig.MaxMemoryCountMB = nil
 			}
 
 			if dbConfig.CacheConfig.RevCacheConfig.ShardCount != nil {
@@ -1116,8 +1122,8 @@ func (dbConfig *DbConfig) deprecatedConfigCacheFallback() (warnings []string) {
 	}
 
 	if dbConfig.DeprecatedRevCacheSize != nil {
-		if dbConfig.CacheConfig.RevCacheConfig.Size == nil {
-			dbConfig.CacheConfig.RevCacheConfig.Size = dbConfig.DeprecatedRevCacheSize
+		if dbConfig.CacheConfig.RevCacheConfig.MaxItemCount == nil {
+			dbConfig.CacheConfig.RevCacheConfig.MaxItemCount = dbConfig.DeprecatedRevCacheSize
 		}
 		warnings = append(warnings, fmt.Sprintf(warningMsgFmt, "rev_cache_size", "cache.rev_cache.size"))
 	}

diff --git a/rest/config_database.go b/rest/config_database.go
@@ -137,8 +137,8 @@ func DefaultDbConfig(sc *StartupConfig, useXattrs bool) *DbConfig {
 		AllowEmptyPassword: base.BoolPtr(false),
 		CacheConfig: &CacheConfig{
 			RevCacheConfig: &RevCacheConfig{
-				Size:       base.Uint32Ptr(db.DefaultRevisionCacheSize),
-				ShardCount: base.Uint16Ptr(db.DefaultRevisionCacheShardCount),
+				MaxItemCount: base.Uint32Ptr(db.DefaultRevisionCacheSize),
+				ShardCount:   base.Uint16Ptr(db.DefaultRevisionCacheShardCount),
 			},
 			ChannelCacheConfig: &ChannelCacheConfig{
 				MaxNumber:            base.IntPtr(db.DefaultChannelCacheMaxNumber),

diff --git a/rest/config_test.go b/rest/config_test.go
@@ -18,6 +18,7 @@ import (
 	"crypto/rsa"
 	"crypto/tls"
 	"crypto/x509"
+	"encoding/json"
 	"errors"
 	"flag"
 	"fmt"
@@ -190,11 +191,11 @@ func TestConfigValidationCache(t *testing.T) {
 
 	require.NotNil(t, config.Databases["db"].CacheConfig.RevCacheConfig)
 	if base.IsEnterpriseEdition() {
-		require.NotNil(t, config.Databases["db"].CacheConfig.RevCacheConfig.Size)
-		assert.Equal(t, 0, int(*config.Databases["db"].CacheConfig.RevCacheConfig.Size))
+		require.NotNil(t, config.Databases["db"].CacheConfig.RevCacheConfig.MaxItemCount)
+		assert.Equal(t, 0, int(*config.Databases["db"].CacheConfig.RevCacheConfig.MaxItemCount))
 	} else {
 		// CE disallowed - should be nil
-		assert.Nil(t, config.Databases["db"].CacheConfig.RevCacheConfig.Size)
+		assert.Nil(t, config.Databases["db"].CacheConfig.RevCacheConfig.MaxItemCount)
 	}
 
 	require.NotNil(t, config.Databases["db"].CacheConfig.ChannelCacheConfig)
@@ -488,7 +489,7 @@ func TestDeprecatedCacheConfig(t *testing.T) {
 	require.Len(t, warnings, 8)
 
 	// Check that the deprecated values have correctly been propagated upto the new config values
-	assert.Equal(t, *dbConfig.CacheConfig.RevCacheConfig.Size, uint32(10))
+	assert.Equal(t, *dbConfig.CacheConfig.RevCacheConfig.MaxItemCount, uint32(10))
 	assert.Equal(t, *dbConfig.CacheConfig.ChannelCacheConfig.ExpirySeconds, 10)
 	assert.Equal(t, *dbConfig.CacheConfig.ChannelCacheConfig.MinLength, 10)
 	assert.Equal(t, *dbConfig.CacheConfig.ChannelCacheConfig.MaxLength, 10)
@@ -507,7 +508,7 @@ func TestDeprecatedCacheConfig(t *testing.T) {
 
 	// Set A Couple Deprecated Values AND Their New Counterparts
 	dbConfig.DeprecatedRevCacheSize = base.Uint32Ptr(10)
-	dbConfig.CacheConfig.RevCacheConfig.Size = base.Uint32Ptr(20)
+	dbConfig.CacheConfig.RevCacheConfig.MaxItemCount = base.Uint32Ptr(20)
 	dbConfig.CacheConfig.DeprecatedEnableStarChannel = base.BoolPtr(false)
 	dbConfig.CacheConfig.ChannelCacheConfig.EnableStarChannel = base.BoolPtr(true)
 
@@ -518,7 +519,7 @@ func TestDeprecatedCacheConfig(t *testing.T) {
 	require.Len(t, warnings, 2)
 
 	// Check that the deprecated value has been ignored as the new value is the priority
-	assert.Equal(t, *dbConfig.CacheConfig.RevCacheConfig.Size, uint32(20))
+	assert.Equal(t, *dbConfig.CacheConfig.RevCacheConfig.MaxItemCount, uint32(20))
 	assert.Equal(t, *dbConfig.CacheConfig.ChannelCacheConfig.EnableStarChannel, true)
 }
 
@@ -3067,3 +3068,42 @@ func TestNotFoundOnInvalidDatabase(t *testing.T) {
 		assert.Equal(c, 1, len(rt.ServerContext().dbConfigs))
 	}, time.Second*10, time.Millisecond*100)
 }
+
+func TestRevCacheMemoryLimitConfig(t *testing.T) {
+	rt := NewRestTester(t, &RestTesterConfig{
+		CustomTestBucket: base.GetTestBucket(t),
+		PersistentConfig: true,
+	})
+	defer rt.Close()
+
+	dbConfig := rt.NewDbConfig()
+	RequireStatus(t, rt.CreateDatabase("db1", dbConfig), http.StatusCreated)
+
+	resp := rt.SendAdminRequest(http.MethodGet, "/db1/_config", "")
+	RequireStatus(t, resp, http.StatusOK)
+
+	require.NoError(t, json.Unmarshal(resp.BodyBytes(), &dbConfig))
+	assert.Nil(t, dbConfig.CacheConfig)
+
+	dbConfig.CacheConfig = &CacheConfig{}
+	dbConfig.CacheConfig.RevCacheConfig = &RevCacheConfig{
+		MaxItemCount:     base.Uint32Ptr(100),
+		MaxMemoryCountMB: base.Uint32Ptr(4),
+	}
+	RequireStatus(t, rt.UpsertDbConfig("db1", dbConfig), http.StatusCreated)
+
+	resp = rt.SendAdminRequest(http.MethodGet, "/db1/_config", "")
+	RequireStatus(t, resp, http.StatusOK)
+
+	// empty db config struct
+	dbConfig = DbConfig{}
+	require.NoError(t, json.Unmarshal(resp.BodyBytes(), &dbConfig))
+	assert.NotNil(t, dbConfig.CacheConfig)
+
+	assert.Equal(t, uint32(100), *dbConfig.CacheConfig.RevCacheConfig.MaxItemCount)
+	if base.IsEnterpriseEdition() {
+		assert.Equal(t, uint32(4), *dbConfig.CacheConfig.RevCacheConfig.MaxMemoryCountMB)
+	} else {
+		assert.Nil(t, dbConfig.CacheConfig.RevCacheConfig.MaxMemoryCountMB)
+	}
+}
diff --git a/rest/revocation_test.go b/rest/revocation_test.go
@@ -1087,7 +1087,7 @@ func TestRevocationsWithQueryLimit(t *testing.T) {
 			QueryPaginationLimit: base.IntPtr(2),
 			CacheConfig: &CacheConfig{
 				RevCacheConfig: &RevCacheConfig{
-					Size: base.Uint32Ptr(0),
+					MaxItemCount: base.Uint32Ptr(0),
 				},
 				ChannelCacheConfig: &ChannelCacheConfig{
 					MaxNumber: base.IntPtr(0),
@@ -1176,7 +1176,7 @@ func TestRevocationsWithQueryLimitChangesLimit(t *testing.T) {
 			QueryPaginationLimit: base.IntPtr(2),
 			CacheConfig: &CacheConfig{
 				RevCacheConfig: &RevCacheConfig{
-					Size: base.Uint32Ptr(0),
+					MaxItemCount: base.Uint32Ptr(0),
 				},
 				ChannelCacheConfig: &ChannelCacheConfig{
 					MaxNumber: base.IntPtr(0),
@@ -1227,7 +1227,7 @@ func TestRevocationUserHasDocAccessDocNotFound(t *testing.T) {
 			QueryPaginationLimit: base.IntPtr(2),
 			CacheConfig: &CacheConfig{
 				RevCacheConfig: &RevCacheConfig{
-					Size: base.Uint32Ptr(0),
+					MaxItemCount: base.Uint32Ptr(0),
 				},
 				ChannelCacheConfig: &ChannelCacheConfig{
 					MaxNumber: base.IntPtr(0),

diff --git a/rest/server_context.go b/rest/server_context.go
@@ -1164,8 +1164,11 @@ func dbcOptionsFromConfig(ctx context.Context, sc *ServerContext, config *DbConf
 		}
 
 		if config.CacheConfig.RevCacheConfig != nil {
-			if config.CacheConfig.RevCacheConfig.Size != nil {
-				revCacheOptions.MaxItemCount = *config.CacheConfig.RevCacheConfig.Size
+			if config.CacheConfig.RevCacheConfig.MaxItemCount != nil {
+				revCacheOptions.MaxItemCount = *config.CacheConfig.RevCacheConfig.MaxItemCount
+			}
+			if config.CacheConfig.RevCacheConfig.MaxMemoryCountMB != nil {
+				revCacheOptions.MaxBytes = int64(*config.CacheConfig.RevCacheConfig.MaxMemoryCountMB * 1024 * 1024) // Convert MB input to bytes
 			}
 			if config.CacheConfig.RevCacheConfig.ShardCount != nil {
 				revCacheOptions.ShardCount = *config.CacheConfig.RevCacheConfig.ShardCount