From 2f5dcd6585140f9dfb6d5fae8fc15d772f2e6ff6 Mon Sep 17 00:00:00 2001 From: Gregory Newman-Smith <109068393+gregns1@users.noreply.github.com> Date: Fri, 6 Sep 2024 17:22:22 +0100 Subject: [PATCH] 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 --------- Signed-off-by: Gregory Newman-Smith --- docs/api/components/schemas.yaml | 7 ++++ rest/access_test.go | 2 +- rest/adminapitest/admin_api_test.go | 2 +- rest/bootstrap_test.go | 4 +-- rest/config.go | 18 ++++++---- rest/config_database.go | 4 +-- rest/config_test.go | 52 +++++++++++++++++++++++++---- rest/revocation_test.go | 6 ++-- rest/server_context.go | 7 ++-- 9 files changed, 79 insertions(+), 23 deletions(-) diff --git a/docs/api/components/schemas.yaml b/docs/api/components/schemas.yaml index adf0396cd5..437e9efd43 100644 --- 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 index 116399e229..86b870fe93 100644 --- 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 index 8be26ea55c..42edfef91f 100644 --- 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 index 1ec82b6034..a712905f0d 100644 --- 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 index 984f3d211f..64b7fafee5 100644 --- 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 index 9fc2d20f13..6d4f97afbd 100644 --- 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 index 9f1aa44345..bb37d240b8 100644 --- 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 index 563024680a..d0bab3e9fa 100644 --- 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 index 261269449c..2c68ea90c1 100644 --- 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