From 28ba253f68d6c037a422b8cc97d789ff5d072927 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Mon, 11 Sep 2023 15:14:50 -0400 Subject: [PATCH 1/9] CBG-3277 add a stat for mismatched bucket names in dbconfigs --- base/stats.go | 33 ++++++++++++++++++++++++----- base/stats_descriptions.go | 5 +++++ rest/adminapitest/admin_api_test.go | 29 +++++++++++++++++++++---- rest/config.go | 1 + 4 files changed, 59 insertions(+), 9 deletions(-) diff --git a/base/stats.go b/base/stats.go index c735dfc29a..3aa7968ee9 100644 --- a/base/stats.go +++ b/base/stats.go @@ -30,6 +30,7 @@ const ( NamespaceKey = "sgw" ResourceUtilizationSubsystem = "resource_utilization" + ErrorsSubsystem = "errors" SubsystemCacheKey = "cache" SubsystemDatabaseKey = "database" @@ -79,6 +80,7 @@ const ( StatAddedVersion3dot0dot0 = "3.0.0" StatAddedVersion3dot1dot0 = "3.1.0" + StatAddedVersion3dot1dot2 = "3.1.2" StatAddedVersion3dot2dot0 = "3.2.0" StatDeprecatedVersionNotDeprecated = "" @@ -105,15 +107,15 @@ var SyncGatewayStats *SgwStats var SkipPrometheusStatsRegistration bool func NewSyncGatewayStats() (*SgwStats, error) { + globalStats, err := newGlobalStat() + if err != nil { + return nil, err + } sgwStats := SgwStats{ - GlobalStats: &GlobalStat{}, + GlobalStats: globalStats, DbStats: map[string]*DbStats{}, } - err := sgwStats.GlobalStats.initResourceUtilizationStats() - if err != nil { - return nil, err - } err = sgwStats.initReplicationStats() if err != nil { return nil, err @@ -158,6 +160,22 @@ func (s *SgwStats) String() string { type GlobalStat struct { ResourceUtilization *ResourceUtilization `json:"resource_utilization"` + ErrorStat *ErrorStat `json:"errors"` +} + +func newGlobalStat() (*GlobalStat, error) { + g := &GlobalStat{ + ErrorStat: &ErrorStat{}, + } + err := g.initResourceUtilizationStats() + + if err != nil { + return nil, err + } + + g.ErrorStat.DatabaseBucketMismatches, err = NewIntStat(ErrorsSubsystem, "database_config_bucket_mistmatches", StatUnitBytes, DatabaseBucketMistmatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) + + return g, err } func (g *GlobalStat) initResourceUtilizationStats() error { @@ -303,6 +321,11 @@ type ResourceUtilization struct { Uptime *SgwDurStat `json:"uptime"` } +type ErrorStat struct { + // The number of times a database config is found in the wrong bucket + DatabaseBucketMismatches *SgwIntStat `json:"database_config_bucket_mistmatches"` +} + type DbStats struct { dbName string CacheStats *CacheStats `json:"cache,omitempty"` diff --git a/base/stats_descriptions.go b/base/stats_descriptions.go index 4f8fb58e33..18bbac51d1 100644 --- a/base/stats_descriptions.go +++ b/base/stats_descriptions.go @@ -67,6 +67,11 @@ const ( UptimeDesc = "The total uptime." ) +// error stat +const ( + DatabaseBucketMistmatchesDesc = "The total number of times a database config that is found to mismatch the bucket listed in the config is polled." +) + // cache stats descriptions const ( AbandonedSequencesDesc = "The total number of skipped sequences that were not found after 60 minutes and were abandoned." diff --git a/rest/adminapitest/admin_api_test.go b/rest/adminapitest/admin_api_test.go index 2c725effd9..7f467cddc0 100644 --- a/rest/adminapitest/admin_api_test.go +++ b/rest/adminapitest/admin_api_test.go @@ -1599,7 +1599,7 @@ func TestBadConfigInsertionToBucket(t *testing.T) { // assert that a request to the database fails with correct error message resp := rt.SendAdminRequest(http.MethodGet, "/db1/_config", "") rest.RequireStatus(t, resp, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), "Must update database config immediately") + assert.Contains(t, resp.Body.String(), "You must update database config immediately") } // TestMismatchedBucketNameOnDbConfigUpdate: @@ -1635,6 +1635,7 @@ func TestMismatchedBucketNameOnDbConfigUpdate(t *testing.T) { // assert request fails resp = rt.ReplaceDbConfig("db1", dbConfig) rest.RequireStatus(t, resp, http.StatusNotFound) + base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) } // TestMultipleBucketWithBadDbConfigScenario1: @@ -1643,6 +1644,9 @@ func TestMismatchedBucketNameOnDbConfigUpdate(t *testing.T) { func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { base.TestsRequireBootstrapConnection(t) base.RequireNumTestBuckets(t, 3) + defer func() { + base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) + }() tb1 := base.GetPersistentTestBucket(t) defer tb1.Close() tb2 := base.GetPersistentTestBucket(t) @@ -1660,12 +1664,12 @@ func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { config.Bootstrap.ConfigGroupID = groupID }, }) - defer rt1.Close() // create a db config that has bucket C in the config and persist to rt1 bucket dbConfig := rt1.NewDbConfig() dbConfig.Name = "db1" rt1.PersistDbConfigToBucket(dbConfig, tb3.GetName()) + defer rt1.Close() rt2 := rest.NewRestTester(t, &rest.RestTesterConfig{ CustomTestBucket: tb2, @@ -1681,6 +1685,7 @@ func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { dbConfig = rt2.NewDbConfig() dbConfig.Name = "db1" rt2.PersistDbConfigToBucket(dbConfig, tb3.GetName()) + base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) rt3 := rest.NewRestTester(t, &rest.RestTesterConfig{ PersistentConfig: true, @@ -1711,13 +1716,17 @@ func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { // assert a request to the db fails with correct error message resp := rt3.SendAdminRequest(http.MethodGet, "/db1/_config", "") rest.RequireStatus(t, resp, http.StatusNotFound) - assert.Contains(t, resp.Body.String(), "Must update database config immediately") + assert.Contains(t, resp.Body.String(), "You must update database config immediately") } // TestMultipleBucketWithBadDbConfigScenario2: // - create bucketA and bucketB with db configs that that both list bucket name as bucketA // - start a new rest tester and assert that invalid db config is picked up and the valid one is also picked up func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { + defer func() { + base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) + }() + base.TestsRequireBootstrapConnection(t) base.RequireNumTestBuckets(t, 3) @@ -1734,11 +1743,12 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { config.Bootstrap.ConfigGroupID = "60ce5544-c368-4b08-b0ed-4ca3b37973f9" }, }) + defer rt1.Close() + // create a db config pointing to bucket C and persist to bucket A dbConfig := rt1.NewDbConfig() dbConfig.Name = "db1" rt1.PersistDbConfigToBucket(dbConfig, rt1.CustomTestBucket.GetName()) - defer rt1.Close() rt2 := rest.NewRestTester(t, &rest.RestTesterConfig{ CustomTestBucket: tb2, @@ -1750,6 +1760,7 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { }) defer rt2.Close() + require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // create a db config pointing to bucket C and persist to bucket B dbConfig = rt2.NewDbConfig() dbConfig.Name = "db1" @@ -1773,6 +1784,8 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { }, 200, 1000) require.NoError(t, err) + base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) + // assert that there is a valid database picked up as the invalid configs have this rest tester backing bucket err = rt3.WaitForConditionWithOptions(func() bool { validDatabase := rt3.ServerContext().AllDatabases() @@ -1787,6 +1800,10 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { // - persist that db config to another bucket // - assert that is picked up as an invalid db config func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { + defer func() { + base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) + }() + base.TestsRequireBootstrapConnection(t) tb1 := base.GetPersistentTestBucket(t) @@ -1804,6 +1821,7 @@ func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { }) defer rt.Close() + require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // create a new db dbConfig := rt.NewDbConfig() dbConfig.Name = "db1" @@ -1829,12 +1847,15 @@ func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { // add the config to the other bucket rt.InsertDbConfigToBucket(&persistedConfig, tb2.GetName()) + require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // assert the config is picked as invalid db config err = rt.WaitForConditionWithOptions(func() bool { invalidDatabases := rt.ServerContext().AllInvalidDatabases() return len(invalidDatabases) == 1 }, 200, 1000) require.NoError(t, err) + + base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) } func TestResyncStopUsingDCPStream(t *testing.T) { diff --git a/rest/config.go b/rest/config.go index a4a0651ade..febb663af9 100644 --- a/rest/config.go +++ b/rest/config.go @@ -308,6 +308,7 @@ func (d *invalidDatabaseConfigs) addInvalidDatabase(ctx context.Context, dbname // already logged this entry at warning so need to log at info now base.InfofCtx(ctx, base.KeyConfig, logMessage) } + base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Add(1) } func (d *invalidDatabaseConfigs) exists(dbname string) (*invalidConfigInfo, bool) { From 0083b831c2ad4c3b6051f5e0603d1a604c03b99a Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Wed, 13 Sep 2023 08:19:59 -0400 Subject: [PATCH 2/9] Improve assertion message --- rest/utilities_testing.go | 11 +++++++---- 1 file changed, 7 insertions(+), 4 deletions(-) diff --git a/rest/utilities_testing.go b/rest/utilities_testing.go index 43a1b2ded7..7cbdd89555 100644 --- a/rest/utilities_testing.go +++ b/rest/utilities_testing.go @@ -2509,13 +2509,16 @@ func DropAllTestIndexes(t *testing.T, tb *base.TestBucket) { } } -func (sc *ServerContext) RequireInvalidDatabaseConfigNames(t *testing.T, dbNames []string) { +func (sc *ServerContext) RequireInvalidDatabaseConfigNames(t *testing.T, expectedDbNames []string) { sc.invalidDatabaseConfigTracking.m.RLock() defer sc.invalidDatabaseConfigTracking.m.RUnlock() - require.Equal(t, len(dbNames), len(sc.invalidDatabaseConfigTracking.dbNames)) - for _, v := range dbNames { - require.NotNil(t, sc.invalidDatabaseConfigTracking.dbNames[v]) + + dbNames := make([]string, 0, len(sc.invalidDatabaseConfigTracking.dbNames)) + + for name := range sc.invalidDatabaseConfigTracking.dbNames { + dbNames = append(dbNames, name) } + require.EqualValues(t, expectedDbNames, dbNames) } // Calls DropAllIndexes to remove all indexes, then restores the primary index for TestBucketPool readier requirements From 1299ddf179b84c9dfd0af7b7687f93ed1d381fba Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Wed, 13 Sep 2023 09:36:03 -0400 Subject: [PATCH 3/9] fix panic if bootstrap polling removes a database in memory while a REST API call is happening - fix a race where the invalid status wouldn't get cleared: - config polling gets data - PUT config - log about config error / clear config error - this config wouldn't be re-read because it would CAS match - remove assertions to stats, because they are global and with a bucket pool, configs from previous tests will contaiminate the test harness --- rest/adminapitest/admin_api_test.go | 19 ------------------- rest/config.go | 22 +++++++++++++++++----- 2 files changed, 17 insertions(+), 24 deletions(-) diff --git a/rest/adminapitest/admin_api_test.go b/rest/adminapitest/admin_api_test.go index 7f467cddc0..396b39c772 100644 --- a/rest/adminapitest/admin_api_test.go +++ b/rest/adminapitest/admin_api_test.go @@ -1635,7 +1635,6 @@ func TestMismatchedBucketNameOnDbConfigUpdate(t *testing.T) { // assert request fails resp = rt.ReplaceDbConfig("db1", dbConfig) rest.RequireStatus(t, resp, http.StatusNotFound) - base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) } // TestMultipleBucketWithBadDbConfigScenario1: @@ -1644,9 +1643,6 @@ func TestMismatchedBucketNameOnDbConfigUpdate(t *testing.T) { func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { base.TestsRequireBootstrapConnection(t) base.RequireNumTestBuckets(t, 3) - defer func() { - base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) - }() tb1 := base.GetPersistentTestBucket(t) defer tb1.Close() tb2 := base.GetPersistentTestBucket(t) @@ -1685,7 +1681,6 @@ func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { dbConfig = rt2.NewDbConfig() dbConfig.Name = "db1" rt2.PersistDbConfigToBucket(dbConfig, tb3.GetName()) - base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) rt3 := rest.NewRestTester(t, &rest.RestTesterConfig{ PersistentConfig: true, @@ -1723,10 +1718,6 @@ func TestMultipleBucketWithBadDbConfigScenario1(t *testing.T) { // - create bucketA and bucketB with db configs that that both list bucket name as bucketA // - start a new rest tester and assert that invalid db config is picked up and the valid one is also picked up func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { - defer func() { - base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) - }() - base.TestsRequireBootstrapConnection(t) base.RequireNumTestBuckets(t, 3) @@ -1760,7 +1751,6 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { }) defer rt2.Close() - require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // create a db config pointing to bucket C and persist to bucket B dbConfig = rt2.NewDbConfig() dbConfig.Name = "db1" @@ -1784,8 +1774,6 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { }, 200, 1000) require.NoError(t, err) - base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) - // assert that there is a valid database picked up as the invalid configs have this rest tester backing bucket err = rt3.WaitForConditionWithOptions(func() bool { validDatabase := rt3.ServerContext().AllDatabases() @@ -1800,10 +1788,6 @@ func TestMultipleBucketWithBadDbConfigScenario2(t *testing.T) { // - persist that db config to another bucket // - assert that is picked up as an invalid db config func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { - defer func() { - base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Set(0) - }() - base.TestsRequireBootstrapConnection(t) tb1 := base.GetPersistentTestBucket(t) @@ -1821,7 +1805,6 @@ func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { }) defer rt.Close() - require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // create a new db dbConfig := rt.NewDbConfig() dbConfig.Name = "db1" @@ -1847,7 +1830,6 @@ func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { // add the config to the other bucket rt.InsertDbConfigToBucket(&persistedConfig, tb2.GetName()) - require.Equal(t, int64(0), base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value()) // assert the config is picked as invalid db config err = rt.WaitForConditionWithOptions(func() bool { invalidDatabases := rt.ServerContext().AllInvalidDatabases() @@ -1855,7 +1837,6 @@ func TestMultipleBucketWithBadDbConfigScenario3(t *testing.T) { }, 200, 1000) require.NoError(t, err) - base.RequireWaitForStat(t, base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Value, 1) } func TestResyncStopUsingDCPStream(t *testing.T) { diff --git a/rest/config.go b/rest/config.go index febb663af9..3b8a8610a7 100644 --- a/rest/config.go +++ b/rest/config.go @@ -1369,6 +1369,7 @@ func (sc *ServerContext) fetchAndLoadConfigs(ctx context.Context, isInitialStart } for dbName, fetchedConfig := range fetchedConfigs { if dbConfig, ok := sc.dbConfigs[dbName]; ok && dbConfig.cfgCas >= fetchedConfig.cfgCas { + sc.invalidDatabaseConfigTracking.remove(dbName) base.DebugfCtx(ctx, base.KeyConfig, "Database %q bucket %q config has not changed since last update", fetchedConfig.Name, *fetchedConfig.Bucket) delete(fetchedConfigs, dbName) } @@ -1388,7 +1389,7 @@ func (sc *ServerContext) fetchAndLoadConfigs(ctx context.Context, isInitialStart for _, dbName := range deletedDatabases { // It's possible that the "deleted" database was not written to the server until after sc.FetchConfigs had returned... // we'll need to pay for the cost of getting the config again now that we've got the write lock to double-check this db is definitely ok to remove... - found, _, err := sc.fetchDatabase(ctx, dbName) + found, _, err := sc._fetchDatabase(ctx, dbName) if err != nil { base.InfofCtx(ctx, base.KeyConfig, "Error fetching config for database %q to check whether we need to remove it: %v", dbName, err) } @@ -1416,15 +1417,13 @@ func (sc *ServerContext) fetchAndLoadDatabaseSince(ctx context.Context, dbName s } func (sc *ServerContext) fetchAndLoadDatabase(nonContextStruct base.NonCancellableContext, dbName string) (found bool, err error) { - sc.lock.Lock() - defer sc.lock.Unlock() return sc._fetchAndLoadDatabase(nonContextStruct, dbName) } // _fetchAndLoadDatabase will attempt to find the given database name first in a matching bucket name, // but then fall back to searching through configs in each bucket to try and find a config. func (sc *ServerContext) _fetchAndLoadDatabase(nonContextStruct base.NonCancellableContext, dbName string) (found bool, err error) { - found, dbConfig, err := sc.fetchDatabase(nonContextStruct.Ctx, dbName) + found, dbConfig, err := sc._fetchDatabase(nonContextStruct.Ctx, dbName) if err != nil || !found { return false, err } @@ -1492,6 +1491,13 @@ func (sc *ServerContext) findBucketWithCallback(callback func(bucket string) (ex } func (sc *ServerContext) fetchDatabase(ctx context.Context, dbName string) (found bool, dbConfig *DatabaseConfig, err error) { + // fetch will update the databses + sc.lock.Lock() + defer sc.lock.Lock() + return sc._fetchDatabase(ctx, dbName) +} + +func (sc *ServerContext) _fetchDatabase(ctx context.Context, dbName string) (found bool, dbConfig *DatabaseConfig, err error) { // loop code moved to foreachDbConfig var cnf DatabaseConfig callback := func(bucket string) (exit bool, err error) { @@ -1525,7 +1531,7 @@ func (sc *ServerContext) fetchDatabase(ctx context.Context, dbName string) (foun // bucket name we got the config from we need to maker this db context as corrupt. Then remove the context and // in memory representation on the server context. if bucket != *cnf.Bucket { - sc.handleInvalidDatabaseConfig(ctx, bucket, cnf) + sc._handleInvalidDatabaseConfig(ctx, bucket, cnf) return true, fmt.Errorf("mismatch in persisted database bucket name %q vs the actual bucket name %q. Please correct db %q's config, groupID %q.", base.MD(cnf.Bucket), base.MD(bucket), base.MD(cnf.Name), base.MD(sc.Config.Bootstrap.ConfigGroupID)) } bucketCopy := bucket @@ -1553,6 +1559,12 @@ func (sc *ServerContext) fetchDatabase(ctx context.Context, dbName string) (foun } func (sc *ServerContext) handleInvalidDatabaseConfig(ctx context.Context, bucket string, cnf DatabaseConfig) { + sc.lock.Lock() + defer sc.lock.Unlock() + sc._handleInvalidDatabaseConfig(ctx, bucket, cnf) +} + +func (sc *ServerContext) _handleInvalidDatabaseConfig(ctx context.Context, bucket string, cnf DatabaseConfig) { // track corrupt database context sc.invalidDatabaseConfigTracking.addInvalidDatabase(ctx, cnf.Name, cnf, bucket) // don't load config + remove from server context (apart from corrupt database map) From a23601a3ef1849e3d3c614d1f5c8bb2834b51545 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Wed, 13 Sep 2023 09:42:49 -0400 Subject: [PATCH 4/9] Move stats into config subsection --- base/stats.go | 6 +++--- base/stats_descriptions.go | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/base/stats.go b/base/stats.go index 3aa7968ee9..f14da524da 100644 --- a/base/stats.go +++ b/base/stats.go @@ -30,7 +30,7 @@ const ( NamespaceKey = "sgw" ResourceUtilizationSubsystem = "resource_utilization" - ErrorsSubsystem = "errors" + ConfigSubsystem = "config" SubsystemCacheKey = "cache" SubsystemDatabaseKey = "database" @@ -173,7 +173,7 @@ func newGlobalStat() (*GlobalStat, error) { return nil, err } - g.ErrorStat.DatabaseBucketMismatches, err = NewIntStat(ErrorsSubsystem, "database_config_bucket_mistmatches", StatUnitBytes, DatabaseBucketMistmatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) + g.ErrorStat.DatabaseBucketMismatches, err = NewIntStat(ConfigSubsystem, "database_config_bucket_mistmatches", StatUnitBytes, DatabaseBucketMismatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) return g, err } @@ -322,7 +322,7 @@ type ResourceUtilization struct { } type ErrorStat struct { - // The number of times a database config is found in the wrong bucket + // The number of times the bucket specified in a database config doesn't match the bucket it's found in. DatabaseBucketMismatches *SgwIntStat `json:"database_config_bucket_mistmatches"` } diff --git a/base/stats_descriptions.go b/base/stats_descriptions.go index 18bbac51d1..a195a0f2f7 100644 --- a/base/stats_descriptions.go +++ b/base/stats_descriptions.go @@ -69,7 +69,7 @@ const ( // error stat const ( - DatabaseBucketMistmatchesDesc = "The total number of times a database config that is found to mismatch the bucket listed in the config is polled." + DatabaseBucketMismatchesDesc = "The total number of times a database config that is found to mismatch the bucket listed in the config is polled." ) // cache stats descriptions From 6d3fc99b42a780730a7f28ccff6d23b0fc22257c Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 14 Sep 2023 18:10:42 -0400 Subject: [PATCH 5/9] match lock with unlock --- rest/config.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/rest/config.go b/rest/config.go index 3b8a8610a7..1f1f732303 100644 --- a/rest/config.go +++ b/rest/config.go @@ -1493,7 +1493,7 @@ func (sc *ServerContext) findBucketWithCallback(callback func(bucket string) (ex func (sc *ServerContext) fetchDatabase(ctx context.Context, dbName string) (found bool, dbConfig *DatabaseConfig, err error) { // fetch will update the databses sc.lock.Lock() - defer sc.lock.Lock() + defer sc.lock.Unlock() return sc._fetchDatabase(ctx, dbName) } From 5c5ac3c561a13628fc0d77082ae47a25b82013fc Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 14 Sep 2023 19:34:27 -0400 Subject: [PATCH 6/9] add missing fix from a5b25c3bf03113376eb86eea55ebfa38fc824d3d --- rest/adminapitest/admin_api_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/rest/adminapitest/admin_api_test.go b/rest/adminapitest/admin_api_test.go index ae6b4d8108..986c1c81f2 100644 --- a/rest/adminapitest/admin_api_test.go +++ b/rest/adminapitest/admin_api_test.go @@ -3709,7 +3709,7 @@ func TestConfigsIncludeDefaults(t *testing.T) { assert.Equal(t, db.DefaultChannelCacheMaxNumber, *dbConfig.CacheConfig.ChannelCacheConfig.MaxNumber) assert.Equal(t, base.DefaultOldRevExpirySeconds, *dbConfig.OldRevExpirySeconds) assert.Equal(t, false, *dbConfig.StartOffline) - assert.Equal(t, db.DefaultCompactInterval, uint32(*dbConfig.CompactIntervalDays)) + assert.Equal(t, db.DefaultCompactInterval, time.Duration(*dbConfig.CompactIntervalDays)*24*time.Hour) assert.Equal(t, dbConfig.Logging.Console.LogLevel.String(), base.LevelDebug.String()) assert.Equal(t, dbConfig.Logging.Console.LogKeys, []string{base.KeyDCP.String()}) @@ -3726,7 +3726,7 @@ func TestConfigsIncludeDefaults(t *testing.T) { assert.Equal(t, db.DefaultChannelCacheMaxNumber, *runtimeServerConfigDatabase.CacheConfig.ChannelCacheConfig.MaxNumber) assert.Equal(t, base.DefaultOldRevExpirySeconds, *runtimeServerConfigDatabase.OldRevExpirySeconds) assert.Equal(t, false, *runtimeServerConfigDatabase.StartOffline) - assert.Equal(t, db.DefaultCompactInterval, uint32(*runtimeServerConfigDatabase.CompactIntervalDays)) + assert.Equal(t, db.DefaultCompactInterval, time.Duration(*runtimeServerConfigDatabase.CompactIntervalDays)*24*time.Hour) // Test unsupported options tb2 := base.GetTestBucket(t) From e138ae9dd74222cd30889556d65a394f9ece390d Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 14 Sep 2023 19:43:43 -0400 Subject: [PATCH 7/9] Move stat to config, fix spelling --- base/stats.go | 10 +++++----- rest/config.go | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/base/stats.go b/base/stats.go index f14da524da..777ff8e542 100644 --- a/base/stats.go +++ b/base/stats.go @@ -160,12 +160,12 @@ func (s *SgwStats) String() string { type GlobalStat struct { ResourceUtilization *ResourceUtilization `json:"resource_utilization"` - ErrorStat *ErrorStat `json:"errors"` + ConfigStat *ConfigStat `json:"config"` } func newGlobalStat() (*GlobalStat, error) { g := &GlobalStat{ - ErrorStat: &ErrorStat{}, + ConfigStat: &ConfigStat{}, } err := g.initResourceUtilizationStats() @@ -173,7 +173,7 @@ func newGlobalStat() (*GlobalStat, error) { return nil, err } - g.ErrorStat.DatabaseBucketMismatches, err = NewIntStat(ConfigSubsystem, "database_config_bucket_mistmatches", StatUnitBytes, DatabaseBucketMismatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) + g.ConfigStat.DatabaseBucketMismatches, err = NewIntStat(ConfigSubsystem, "database_config_bucket_mismatches", StatUnitBytes, DatabaseBucketMismatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) return g, err } @@ -321,9 +321,9 @@ type ResourceUtilization struct { Uptime *SgwDurStat `json:"uptime"` } -type ErrorStat struct { +type ConfigStat struct { // The number of times the bucket specified in a database config doesn't match the bucket it's found in. - DatabaseBucketMismatches *SgwIntStat `json:"database_config_bucket_mistmatches"` + DatabaseBucketMismatches *SgwIntStat `json:"database_config_bucket_mismatches"` } type DbStats struct { diff --git a/rest/config.go b/rest/config.go index 558666c560..91e8c79132 100644 --- a/rest/config.go +++ b/rest/config.go @@ -308,7 +308,7 @@ func (d *invalidDatabaseConfigs) addInvalidDatabase(ctx context.Context, dbname // already logged this entry at warning so need to log at info now base.InfofCtx(ctx, base.KeyConfig, logMessage) } - base.SyncGatewayStats.GlobalStats.ErrorStat.DatabaseBucketMismatches.Add(1) + base.SyncGatewayStats.GlobalStats.ConfigStat.DatabaseBucketMismatches.Add(1) } func (d *invalidDatabaseConfigs) exists(dbname string) (*invalidConfigInfo, bool) { From 1f90882886f05a07b87ee1bd510ee885284d6e56 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 14 Sep 2023 20:03:38 -0400 Subject: [PATCH 8/9] Apply suggestions from code review Co-authored-by: Adam Fraser --- base/stats_descriptions.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/base/stats_descriptions.go b/base/stats_descriptions.go index a195a0f2f7..7b9af144d8 100644 --- a/base/stats_descriptions.go +++ b/base/stats_descriptions.go @@ -69,7 +69,7 @@ const ( // error stat const ( - DatabaseBucketMismatchesDesc = "The total number of times a database config that is found to mismatch the bucket listed in the config is polled." + DatabaseBucketMismatchesDesc = "The total number of times a database config is polled from a bucket that doesn't match the bucket specified in the database config." ) // cache stats descriptions From 0b586e4bb0142bdb65a52b5c91781520265f2af0 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 14 Sep 2023 20:07:53 -0400 Subject: [PATCH 9/9] Refactor to match existing style --- base/stats.go | 23 ++++++++++++++++------- 1 file changed, 16 insertions(+), 7 deletions(-) diff --git a/base/stats.go b/base/stats.go index 777ff8e542..8e7cc1d718 100644 --- a/base/stats.go +++ b/base/stats.go @@ -164,18 +164,27 @@ type GlobalStat struct { } func newGlobalStat() (*GlobalStat, error) { - g := &GlobalStat{ - ConfigStat: &ConfigStat{}, - } + g := &GlobalStat{} err := g.initResourceUtilizationStats() - if err != nil { return nil, err } + err = g.initConfigStats() + if err != nil { + return nil, err + } + return g, nil +} - g.ConfigStat.DatabaseBucketMismatches, err = NewIntStat(ConfigSubsystem, "database_config_bucket_mismatches", StatUnitBytes, DatabaseBucketMismatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) - - return g, err +func (g *GlobalStat) initConfigStats() error { + configStat := &ConfigStat{} + var err error + configStat.DatabaseBucketMismatches, err = NewIntStat(ConfigSubsystem, "database_config_bucket_mismatches", StatUnitBytes, DatabaseBucketMismatchesDesc, StatAddedVersion3dot1dot2, StatDeprecatedVersionNotDeprecated, StatStabilityCommitted, nil, nil, prometheus.CounterValue, 0) + if err != nil { + return err + } + g.ConfigStat = configStat + return nil } func (g *GlobalStat) initResourceUtilizationStats() error {