[4.0.2] CBG-5007: Split ServerContext mutexes

rest/admin_api.go

@@ -111,10 +111,10 @@ func (h *handler) handleCreateDB() error {
 			SGVersion:  base.ProductVersion.String(),
 		}
 
-		h.server.lock.Lock()
-		defer h.server.lock.Unlock()
+		h.server._databasesLock.Lock()
+		defer h.server._databasesLock.Unlock()
 
-		if _, exists := h.server.databases_[dbName]; exists {
+		if _, exists := h.server._databases[dbName]; exists {
 			return base.HTTPErrorf(http.StatusPreconditionFailed, // what CouchDB returns
 				"Duplicate database name %q", dbName)
 		}
@@ -154,7 +154,7 @@ func (h *handler) handleCreateDB() error {
 			return base.HTTPErrorf(http.StatusInternalServerError, "couldn't save database config: %v", err)
 		}
 		// store the cas in the loaded config after a successful insert
-		h.server.dbConfigs[dbName].cfgCas = cas
+		h.server._dbConfigs[dbName].cfgCas = cas
 	} else {
 		// Intentionally pass in an empty BootstrapConfig to avoid inheriting any credentials or server when running with a legacy config (CBG-1764)
 		if err := config.setup(h.ctx(), dbName, BootstrapConfig{}, nil, nil, false); err != nil {
@@ -820,9 +820,9 @@ func (h *handler) handlePutDbConfig() (err error) {
 	auditDbAuditEnabled(h.ctx(), dbName, previousAuditEnabled, updatedAuditEnabled, updatedAuditEvents)
 	// store the cas in the loaded config after a successful update
 	h.setEtag(updatedDbConfig.Version)
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
-	h.server.dbConfigs[dbName].cfgCas = cas
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
+	h.server._dbConfigs[dbName].cfgCas = cas
 
 	base.Audit(h.ctx(), base.AuditIDUpdateDatabaseConfig, auditFields)
 	return base.HTTPErrorf(http.StatusCreated, "updated")
@@ -1145,8 +1145,8 @@ func (h *handler) handleDeleteCollectionConfigSync() error {
 		return err
 	}
 
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
 
 	// TODO: Dynamic update instead of reload
 	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false, false); err != nil {
@@ -1214,8 +1214,8 @@ func (h *handler) handlePutCollectionConfigSync() error {
 		return err
 	}
 
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
 
 	// TODO: Dynamic update instead of reload
 	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false, false); err != nil {
@@ -1314,8 +1314,8 @@ func (h *handler) handleDeleteCollectionConfigImportFilter() error {
 		return err
 	}
 
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
 
 	// TODO: Dynamic update instead of reload
 	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false, false); err != nil {
@@ -1384,8 +1384,8 @@ func (h *handler) handlePutCollectionConfigImportFilter() error {
 		return err
 	}
 
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
 
 	// TODO: Dynamic update instead of reload
 	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false, false); err != nil {
@@ -1542,7 +1542,7 @@ func (h *handler) handleGetStatus() error {
 		status.Vendor.Version = base.ProductAPIVersion
 	}
 
-	for _, database := range h.server.databases_ {
+	for _, database := range h.server._databases {
 		lastSeq := uint64(0)
 		runState := db.RunStateString[atomic.LoadUint32(&database.State)]
 

rest/api_test.go

@@ -2981,7 +2981,9 @@ func TestCreateDBWithXattrsDisabled(t *testing.T) {
 	resp = rt.CreateDatabase(dbName, dbConfig)
 	RequireStatus(t, resp, http.StatusCreated)
 
-	rt.RestTesterServerContext.dbConfigs[dbName].DatabaseConfig.EnableXattrs = base.Ptr(false)
+	rt.updatePersistedConfig(dbName, func(config *DatabaseConfig) {
+		config.EnableXattrs = base.Ptr(false)
+	})
 
 	_, err := rt.RestTesterServerContext.ReloadDatabase(t.Context(), dbName, false)
 	assert.Error(t, err, errResp)
@@ -3471,8 +3473,10 @@ func TestAllowConflictsConfig(t *testing.T) {
 	require.Equal(t, fmt.Sprintf(`[{"db_name":"%s","bucket":"%s","state":"Online"}]`, rt.GetDatabase().Name, rt.GetDatabase().Bucket.GetName()), resp.Body.String())
 	bucketName := rt.GetDatabase().Bucket.GetName()
 
-	// Attempt to set the `AllowConflicts` property to true in the database configuration.
-	rt.RestTesterServerContext.dbConfigs[dbName].DatabaseConfig.DbConfig.AllowConflicts = base.Ptr(true)
+	// Set the `AllowConflicts` property to true in the persisted database configuration and try to load it (this covers cases where the config value was previously set from an older SGW version that allowed this configuration).
+	rt.updatePersistedConfig(dbName, func(config *DatabaseConfig) {
+		config.DbConfig.AllowConflicts = base.Ptr(true)
+	})
 
 	// Reload the database configuration and verify that an error is returned.
 	_, err := rt.RestTesterServerContext.ReloadDatabase(ctx, dbName, false)
@@ -3515,7 +3519,9 @@ func TestDisableAllowStarChannel(t *testing.T) {
 	RequireStatus(t, resp, http.StatusCreated)
 
 	// Attempting to disable `enable_star_channel`
-	rt.ServerContext().dbConfigs[dbName].DatabaseConfig.CacheConfig.ChannelCacheConfig.EnableStarChannel = base.Ptr(false)
+	rt.updatePersistedConfig(dbName, func(config *DatabaseConfig) {
+		config.CacheConfig.ChannelCacheConfig.EnableStarChannel = base.Ptr(false)
+	})
 
 	// Reloading the database after updating the config
 	_, err := rt.ServerContext().ReloadDatabase(ctx, dbName, false)

rest/config.go

@@ -1659,21 +1659,21 @@ func (sc *ServerContext) fetchAndLoadConfigs(ctx context.Context, isInitialStart
 	// we don't need to do this two-stage lock on initial startup as the REST APIs aren't even online yet.
 	var deletedDatabases []string
 	if !isInitialStartup {
-		sc.lock.RLock()
-		for dbName, _ := range sc.dbRegistry {
+		sc._databasesLock.RLock()
+		for dbName, _ := range sc._dbRegistry {
 			if _, foundMatchingDb := fetchedConfigs[dbName]; !foundMatchingDb {
 				deletedDatabases = append(deletedDatabases, dbName)
 				delete(fetchedConfigs, dbName)
 			}
 		}
 		for dbName, fetchedConfig := range fetchedConfigs {
-			if dbConfig, ok := sc.dbConfigs[dbName]; ok && dbConfig.cfgCas >= fetchedConfig.cfgCas {
+			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)
 			}
 		}
-		sc.lock.RUnlock()
+		sc._databasesLock.RUnlock()
 
 		// nothing to do, we can bail out without needing the write lock
 		if len(deletedDatabases) == 0 && len(fetchedConfigs) == 0 {
@@ -1683,10 +1683,10 @@ func (sc *ServerContext) fetchAndLoadConfigs(ctx context.Context, isInitialStart
 	}
 
 	// we have databases to update/remove
-	sc.lock.Lock()
-	defer sc.lock.Unlock()
+	sc._databasesLock.Lock()
+	defer sc._databasesLock.Unlock()
 	for _, dbName := range deletedDatabases {
-		dbc, ok := sc.databases_[dbName]
+		dbc, ok := sc._databases[dbName]
 		if !ok {
 			base.DebugfCtx(ctx, base.KeyConfig, "Database %q already removed from server context after acquiring write lock - do not need to remove not removing database", base.MD(dbName))
 			continue
@@ -1725,8 +1725,8 @@ func (sc *ServerContext) fetchAndLoadDatabaseSince(ctx context.Context, dbName s
 }
 
 func (sc *ServerContext) fetchAndLoadDatabase(nonContextStruct base.NonCancellableContext, dbName string, forceReload bool) (found bool, err error) {
-	sc.lock.Lock()
-	defer sc.lock.Unlock()
+	sc._databasesLock.Lock()
+	defer sc._databasesLock.Unlock()
 	return sc._fetchAndLoadDatabase(nonContextStruct, dbName, forceReload)
 }
 
@@ -1808,8 +1808,8 @@ 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.Unlock()
+	sc._databasesLock.Lock()
+	defer sc._databasesLock.Unlock()
 	return sc._fetchDatabase(ctx, dbName)
 }
 
@@ -1879,8 +1879,8 @@ func (sc *ServerContext) _fetchDatabase(ctx context.Context, dbName string) (fou
 }
 
 func (sc *ServerContext) handleInvalidDatabaseConfig(ctx context.Context, bucket string, cnf DatabaseConfig) {
-	sc.lock.Lock()
-	defer sc.lock.Unlock()
+	sc._databasesLock.Lock()
+	defer sc._databasesLock.Unlock()
 	sc._handleInvalidDatabaseConfig(ctx, bucket, cnf, nil)
 }
 
@@ -1894,9 +1894,9 @@ func (sc *ServerContext) _handleInvalidDatabaseConfig(ctx context.Context, bucke
 func (sc *ServerContext) bucketNameFromDbName(ctx context.Context, dbName string) (bucketName string, found bool) {
 	// Minimal representation of config struct to be tolerant of invalid database configurations where we still need to find a database name
 	// see if we find the database in-memory first, otherwise fall back to scanning buckets for db configs
-	sc.lock.RLock()
-	dbc, ok := sc.databases_[dbName]
-	sc.lock.RUnlock()
+	sc._databasesLock.RLock()
+	dbc, ok := sc._databases[dbName]
+	sc._databasesLock.RUnlock()
 
 	if ok {
 		return dbc.Bucket.GetName(), true
@@ -1947,7 +1947,7 @@ func (sc *ServerContext) fetchConfigsSince(ctx context.Context, refreshInterval
 		sc.fetchConfigsLastUpdate = time.Now()
 	}
 
-	return sc.dbConfigs, nil
+	return sc._dbConfigs, nil
 }
 
 // GetBucketNames returns a slice of the bucket names associated with the server context
@@ -2063,8 +2063,8 @@ func (sc *ServerContext) _applyConfigs(ctx context.Context, dbNameConfigs map[st
 }
 
 func (sc *ServerContext) applyConfigs(ctx context.Context, dbNameConfigs map[string]DatabaseConfig) (count int) {
-	sc.lock.Lock()
-	defer sc.lock.Unlock()
+	sc._databasesLock.Lock()
+	defer sc._databasesLock.Unlock()
 	return sc._applyConfigs(ctx, dbNameConfigs, false, false)
 }
 
@@ -2103,13 +2103,13 @@ func (sc *ServerContext) _applyConfig(nonContextStruct base.NonCancellableContex
 	}
 
 	// skip if we already have this config loaded, and we've got a cas value to compare with
-	_, exists := sc.dbRegistry[cnf.Name]
+	_, exists := sc._dbRegistry[cnf.Name]
 	if exists {
 		if cnf.cfgCas == 0 {
 			// force an update when the new config's cas was set to zero prior to load
 			base.InfofCtx(ctx, base.KeyConfig, "Forcing update of config for database %q bucket %q", cnf.Name, *cnf.Bucket)
 		} else {
-			if sc.dbConfigs[cnf.Name].cfgCas >= cnf.cfgCas {
+			if sc._dbConfigs[cnf.Name].cfgCas >= cnf.cfgCas {
 				base.DebugfCtx(ctx, base.KeyConfig, "Database %q bucket %q config has not changed since last update", cnf.Name, *cnf.Bucket)
 				return false, nil
 			}
@@ -2210,8 +2210,8 @@ func StartServer(ctx context.Context, config *StartupConfig, sc *ServerContext)
 }
 
 func sharedBucketDatabaseCheck(ctx context.Context, sc *ServerContext) (errors error) {
-	bucketUUIDToDBContext := make(map[string][]*db.DatabaseContext, len(sc.databases_))
-	for _, dbContext := range sc.databases_ {
+	bucketUUIDToDBContext := make(map[string][]*db.DatabaseContext, len(sc._databases))
+	for _, dbContext := range sc._databases {
 		if uuid, err := dbContext.Bucket.UUID(); err == nil {
 			bucketUUIDToDBContext[uuid] = append(bucketUUIDToDBContext[uuid], dbContext)
 		}
@@ -2271,15 +2271,15 @@ func (sc *ServerContext) _findDuplicateCollections(cnf DatabaseConfig) []string
 	// If scopes aren't defined, check the default collection
 	if cnf.Scopes == nil {
 		defaultFQName := base.FullyQualifiedCollectionName(*cnf.Bucket, base.DefaultScope, base.DefaultCollection)
-		existingDbName, ok := sc.collectionRegistry[defaultFQName]
+		existingDbName, ok := sc._collectionRegistry[defaultFQName]
 		if ok && existingDbName != cnf.Name {
 			duplicatedCollections = append(duplicatedCollections, defaultFQName)
 		}
 	} else {
 		for scopeName, scope := range cnf.Scopes {
 			for collectionName, _ := range scope.Collections {
 				fqName := base.FullyQualifiedCollectionName(*cnf.Bucket, scopeName, collectionName)
-				existingDbName, ok := sc.collectionRegistry[fqName]
+				existingDbName, ok := sc._collectionRegistry[fqName]
 				if ok && existingDbName != cnf.Name {
 					duplicatedCollections = append(duplicatedCollections, fqName)
 				}

rest/config_test.go

@@ -2976,7 +2976,7 @@ func TestInvalidDbConfigNoLongerPresentInBucket(t *testing.T) {
 	require.EventuallyWithT(t, func(c *assert.CollectT) {
 		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
 		assert.Equal(c, 1, len(invalidDatabases))
-		assert.Equal(c, 0, len(rt.ServerContext().dbConfigs))
+		assert.Equal(c, 0, len(rt.ServerContext()._dbConfigs))
 	}, time.Second*10, time.Millisecond*100)
 
 	// remove the invalid config from the bucket
@@ -2989,7 +2989,7 @@ func TestInvalidDbConfigNoLongerPresentInBucket(t *testing.T) {
 	require.EventuallyWithT(t, func(c *assert.CollectT) {
 		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
 		assert.Equal(c, 0, len(invalidDatabases))
-		assert.Equal(c, 0, len(rt.ServerContext().dbConfigs))
+		assert.Equal(c, 0, len(rt.ServerContext()._dbConfigs))
 	}, time.Second*10, time.Millisecond*100)
 
 	// create db again, should succeed
@@ -3062,7 +3062,7 @@ func TestNotFoundOnInvalidDatabase(t *testing.T) {
 	require.EventuallyWithT(t, func(c *assert.CollectT) {
 		invalidDatabases := rt.ServerContext().AllInvalidDatabaseNames(t)
 		assert.Equal(c, 0, len(invalidDatabases))
-		assert.Equal(c, 1, len(rt.ServerContext().dbConfigs))
+		assert.Equal(c, 1, len(rt.ServerContext()._dbConfigs))
 	}, time.Second*10, time.Millisecond*100)
 }
 

rest/handler_config_database.go

@@ -83,8 +83,8 @@ func (h *handler) mutateDbConfig(mutator func(*DbConfig) error) error {
 		return err
 	}
 
-	h.server.lock.Lock()
-	defer h.server.lock.Unlock()
+	h.server._databasesLock.Lock()
+	defer h.server._databasesLock.Unlock()
 
 	// TODO: Dynamic update instead of reload
 	if err := h.server._reloadDatabaseWithConfig(h.ctx(), *updatedDbConfig, false, false); err != nil {