CBG-3384 Avoid triggering unnecessary resync when working with default collection

base/bootstrap.go

@@ -42,8 +42,12 @@ type BootstrapConnection interface {
 	// TouchMetadataDocument sets the specified property in a bootstrap metadata document for a given bucket and key.  Used to
 	// trigger CAS update on the document, to block any racing updates. Does not retry on CAS failure.
 	TouchMetadataDocument(ctx context.Context, bucket, key string, property string, value string, cas uint64) (casOut uint64, err error)
-	// KeyExists checks whether the specified key exists
+	// KeyExists checks whether the specified key exists in the bucket's default collection
 	KeyExists(ctx context.Context, bucket, key string) (exists bool, err error)
+	// GetDocument retrieves the document with the specified key from the bucket's default collection.
+	// Returns exists=false if key is not found, returns error for any other error.
+	GetDocument(ctx context.Context, bucket, docID string, rv interface{}) (exists bool, err error)
+
 	// Returns the bootstrap connection's cluster connection as N1QLStore for the specified bucket/scope/collection.
 	// Does NOT establish a bucket connection, the bucketName/scopeName/collectionName is for query scoping only
 	GetClusterN1QLStore(bucketName, scopeName, collectionName string) (*ClusterOnlyN1QLStore, error)
@@ -447,6 +451,7 @@ func (cc *CouchbaseCluster) UpdateMetadataDocument(ctx context.Context, location
 
 }
 
+// KeyExists checks whether a key exists in the default collection for the specified bucket
 func (cc *CouchbaseCluster) KeyExists(ctx context.Context, location, docID string) (exists bool, err error) {
 	if cc == nil {
 		return false, errors.New("nil CouchbaseCluster")
@@ -463,6 +468,34 @@ func (cc *CouchbaseCluster) KeyExists(ctx context.Context, location, docID strin
 	return cc.configPersistence.keyExists(b.DefaultCollection(), docID)
 }
 
+// GetDocument fetches a document from the default collection.  Does not use configPersistence - callers
+// requiring configPersistence handling should use GetMetadataDocument.
+func (cc *CouchbaseCluster) GetDocument(ctx context.Context, bucketName, docID string, rv interface{}) (exists bool, err error) {
+	if cc == nil {
+		return false, errors.New("nil CouchbaseCluster")
+	}
+
+	b, teardown, err := cc.getBucket(ctx, bucketName)
+	if err != nil {
+		return false, err
+	}
+
+	defer teardown()
+
+	getOptions := &gocb.GetOptions{
+		Transcoder: NewSGJSONTranscoder(),
+	}
+	getResult, err := b.DefaultCollection().Get(docID, getOptions)
+	if err != nil {
+		if errors.Is(err, gocb.ErrDocumentNotFound) {
+			return false, nil
+		}
+		return false, err
+	}
+	err = getResult.Content(rv)
+	return true, err
+}
+
 // Close calls teardown for any cached buckets and removes from cachedBucketConnections
 func (cc *CouchbaseCluster) Close() {
 

base/constants_syncdocs.go

@@ -375,6 +375,9 @@ func InitSyncInfo(ds DataStore, metadataID string) (requiresResync bool, err err
 	var syncInfo SyncInfo
 	_, fetchErr := ds.Get(SGSyncInfo, &syncInfo)
 	if IsKeyNotFoundError(ds, fetchErr) {
+		if metadataID == "" {
+			return false, nil
+		}
 		newSyncInfo := &SyncInfo{MetadataID: metadataID}
 		_, addErr := ds.Add(SGSyncInfo, 0, newSyncInfo)
 		if IsCasMismatch(addErr) {
@@ -397,6 +400,11 @@ func InitSyncInfo(ds DataStore, metadataID string) (requiresResync bool, err err
 
 // SetSyncInfo sets syncInfo in a DataStore to the specified metadataID
 func SetSyncInfo(ds DataStore, metadataID string) error {
+
+	// If the metadataID isn't defined, don't persist SyncInfo.  Defensive handling for legacy use cases.
+	if metadataID == "" {
+		return nil
+	}
 	syncInfo := &SyncInfo{
 		MetadataID: metadataID,
 	}

rest/adminapitest/admin_api_test.go

@@ -4434,19 +4434,6 @@ func TestDeleteDatabasePointingAtSameBucketPersistent(t *testing.T) {
 	resp = rest.BootstrapAdminRequest(t, http.MethodPut, "/db2/", fmt.Sprintf(dbConfig, "db2"))
 	resp.RequireStatus(http.StatusCreated)
 
-	// because we moved database - resync is required for the default collection before we're able to bring db2 online
-	resp = rest.BootstrapAdminRequest(t, http.MethodPost, "/db2/_resync?regenerate_sequences=true", "")
-	resp.RequireStatus(http.StatusOK)
-
-	// after resync is done, state will flip back to offline
-	BootstrapWaitForDatabaseState(t, "db2", db.DBOffline)
-
-	// now bring the db online so we're able to check dest factory
-	resp = rest.BootstrapAdminRequest(t, http.MethodPost, "/db2/_online", "")
-	resp.RequireStatus(http.StatusOK)
-
-	BootstrapWaitForDatabaseState(t, "db2", db.DBOnline)
-
 	scopeName := ""
 	collectionNames := []string{}
 	// Validate that deleted database is no longer in dest factory set

rest/config_manager.go

@@ -721,23 +721,32 @@ func (b *bootstrapContext) ComputeMetadataIDForDbConfig(ctx context.Context, con
 // computeMetadataID determines whether the database should use the default metadata storage location (to support configurations upgrading with
 // existing sync metadata in the default collection).  The default metadataID is only used when all of the following
 // conditions are met:
-//  1. The default metadataID isn't already in use by another database
-//  2. The database includes _default._default
-//  3. The _default._default collection isn't already associated with a different metadata ID (_sync:syncInfo is not present)
-//  4. The _default._default collection has legacy data (_sync:seq is present)
+//  1. There isn't already a metadataID defined for the database name  in another config group
+//  2. The default metadataID isn't already in use by another database in the registry
+//  3. The database includes _default._default
+//  4. The _default._default collection isn't already associated with a different metadata ID (syncInfo document is not present, or has a value of defaultMetadataID)
 func (b *bootstrapContext) computeMetadataID(ctx context.Context, registry *GatewayRegistry, config *DbConfig) string {
 
 	standardMetadataID := b.standardMetadataID(config.Name)
 
-	// If the default metadata ID is already in use in the registry, use standard ID
+	// If there's already a metadataID assigned to this database in the registry (including other config groups), use that
+	defaultMetadataIDInUse := false
 	for _, cg := range registry.ConfigGroups {
-		for _, db := range cg.Databases {
+		for dbName, db := range cg.Databases {
+			if dbName == config.Name {
+				return db.MetadataID
+			}
 			if db.MetadataID == defaultMetadataID {
-				return standardMetadataID
+				defaultMetadataIDInUse = true
 			}
 		}
 	}
 
+	// If the default metadata ID is already in use in the registry by a different database, use standard ID.
+	if defaultMetadataIDInUse {
+		return standardMetadataID
+	}
+
 	// If the database config doesn't include _default._default, use standard ID
 	if config.Scopes != nil {
 		defaultFound := false
@@ -753,21 +762,19 @@ func (b *bootstrapContext) computeMetadataID(ctx context.Context, registry *Gate
 		}
 	}
 
+	// If _default._default is already associated with a non-default metadataID, use the standard ID
 	bucketName := config.GetBucketName()
-	exists, err := b.Connection.KeyExists(ctx, bucketName, base.SGSyncInfo)
+	var syncInfo base.SyncInfo
+	exists, err := b.Connection.GetDocument(ctx, bucketName, base.SGSyncInfo, &syncInfo)
 	if err != nil {
-		base.WarnfCtx(ctx, "Error checking whether metadataID is already defined for default collection - using standard metadataID.  Error: %v", err)
-		return standardMetadataID
-	}
-	if exists {
+		base.WarnfCtx(ctx, "Error checking syncInfo metadataID in default collection - using standard metadataID.  Error: %v", err)
 		return standardMetadataID
 	}
 
-	// If legacy _sync:seq doesn't exist, use the standard ID
-	legacySyncSeqExists, _ := b.Connection.KeyExists(ctx, bucketName, base.DefaultMetadataKeys.SyncSeqKey())
-	if !legacySyncSeqExists {
+	if exists && syncInfo.MetadataID != defaultMetadataID {
 		return standardMetadataID
 	}
+
 	return defaultMetadataID
 
 }

rest/config_manager_test.go

@@ -89,11 +89,11 @@ func TestComputeMetadataID(t *testing.T) {
 	defaultVersion := "1-abc"
 	defaultDbConfig := makeDbConfig(tb.GetName(), dbName, nil)
 
-	// No sync data in default collection, so should use standard ID
+	// Use defaultMetadataID if database targets the default collection
 	metadataID := bootstrapContext.computeMetadataID(ctx, registry, &defaultDbConfig)
-	assert.Equal(t, standardMetadataID, metadataID)
+	assert.Equal(t, defaultMetadataID, metadataID)
 
-	// Set _sync:seq in default collection, verify computeMetadataID returns default ID
+	// Set _sync:seq in default collection, verify computeMetadataID still returns default ID
 	defaultStore := tb.Bucket.DefaultDataStore()
 	syncSeqKey := base.DefaultMetadataKeys.SyncSeqKey()
 	_, err = defaultStore.Incr(syncSeqKey, 1, 0, 0)
@@ -124,6 +124,21 @@ func TestComputeMetadataID(t *testing.T) {
 	metadataID = bootstrapContext.computeMetadataID(ctx, registry, &defaultDbConfig)
 	assert.Equal(t, defaultMetadataID, metadataID)
 
+	// If the database has been assigned a metadataID in another config group, that should be used
+	multiConfigGroupDbName := "multiConfigGroupDb"
+	existingDbConfigOtherConfigGroup := makeDbConfig(tb.GetName(), multiConfigGroupDbName, nil)
+	existingDatabaseConfigOtherConfigGroup := &DatabaseConfig{
+		DbConfig:   existingDbConfigOtherConfigGroup,
+		Version:    defaultVersion,
+		MetadataID: multiConfigGroupDbName,
+	}
+	_, err = registry.upsertDatabaseConfig(ctx, "differentConfigGroup", existingDatabaseConfigOtherConfigGroup)
+	require.NoError(t, err)
+
+	newDbConfig := makeDbConfig(tb.GetName(), multiConfigGroupDbName, nil)
+	metadataID = bootstrapContext.computeMetadataID(ctx, registry, &newDbConfig)
+	require.Equal(t, multiConfigGroupDbName, metadataID)
+
 	// Single, non-default collection should use standard metadata ID
 	namedOnlyScopesConfig := ScopesConfig{base.DefaultScope: ScopeConfig{map[string]*CollectionConfig{"collection1": {}}}}
 	defaultDbConfig.Scopes = namedOnlyScopesConfig