Skip to content

Commit

Permalink
CBG-3384 Avoid triggering unnecessary resync when working with defaul…
Browse files Browse the repository at this point in the history
…t collection (#6504)

* CBG-3384 Modify computation of metadataID

- Don’t persist syncInfo for empty metadataID (legacy config)
- Modify requirements for assignment of default metadata ID to a database:
1. Remove requirement for presence of _sync:seq in default collection
2. Allow assignment when default collection’s syncInfo is using defaultMetadataID

These avoid triggering resync for databases using only the default collection, in legacy and non-legacy mode.

If database already exists in registry, new databases (in different config groups) should use the same metadataID.
  • Loading branch information
adamcfraser authored and bbrks committed Mar 28, 2024
1 parent 441c3c0 commit fe0a891
Show file tree
Hide file tree
Showing 7 changed files with 261 additions and 36 deletions.
35 changes: 34 additions & 1 deletion base/bootstrap.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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")
Expand All @@ -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() {

Expand Down
8 changes: 8 additions & 0 deletions base/constants_syncdocs.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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,
}
Expand Down
13 changes: 0 additions & 13 deletions rest/adminapitest/admin_api_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4499,19 +4499,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
Expand Down
37 changes: 22 additions & 15 deletions rest/config_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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

}
Expand Down
21 changes: 18 additions & 3 deletions rest/config_manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
Loading

0 comments on commit fe0a891

Please sign in to comment.