CBG-4614 Add toggle for using separate principal indexes (#7499)

Author: torcolvin

db/database.go

@@ -175,7 +175,6 @@ type DatabaseContextOptions struct {
 	GroupID                       string
 	JavascriptTimeout             time.Duration // Max time the JS functions run for (ie. sync fn, import filter)
 	UseLegacySyncDocsIndex        bool
-	Serverless                    bool // If running in serverless mode
 	Scopes                        ScopesOptions
 	MetadataStore                 base.DataStore // If set, use this location/connection for SG metadata storage - if not set, metadata is stored using the same location/connection as the bucket used for data storage.
 	MetadataID                    string         // MetadataID used for metadata storage
@@ -869,12 +868,9 @@ func (context *DatabaseContext) Authenticator(ctx context.Context) *auth.Authent
 	return authenticator
 }
 
-func (context *DatabaseContext) IsServerless() bool {
-	return context.Options.Serverless
-}
-
+// UseLegacySyncDocsIndex returns true there is an index over all docs starting with _sync, or if there are individual indexes for users and roles.
 func (context *DatabaseContext) UseLegacySyncDocsIndex() bool {
-	return true // switch in CBG-4614
+	return context.Options.UseLegacySyncDocsIndex
 }
 
 // Makes a Database object given its name and bucket.
@@ -1013,60 +1009,31 @@ func (c *DatabaseCollection) processForEachDocIDResults(ctx context.Context, cal
 // Returns the IDs of all users and roles, including deleted Roles
 func (db *DatabaseContext) AllPrincipalIDs(ctx context.Context) (users, roles []string, err error) {
 
-	if !db.IsServerless() || db.Options.UseViews {
+	if db.UseLegacySyncDocsIndex() || db.Options.UseViews {
 		return db.getAllPrincipalIDsSyncDocs(ctx)
 	}
 
-	// If running in Serverless mode, we can leverage `users` and `roles` index
-	// to fetch users and roles
-	usersCh := db.getUserNamesInBackground(ctx)
-	rolesCh := db.getRoleIDsInBackground(ctx)
-
-	userData := <-usersCh
-	if userData.err != nil {
-		return nil, nil, userData.err
-	}
-	users = userData.value
-
-	rolesData := <-rolesCh
-	if rolesData.err != nil {
-		return nil, nil, rolesData.err
-	}
-	roles = rolesData.value
-
-	return users, roles, err
-}
-
-// used to send users/roles data from background fetch
-type data struct {
-	value []string
-	err   error
-}
-
-func (db *DatabaseContext) getUserNamesInBackground(ctx context.Context) <-chan data {
-	ch := make(chan data, 1)
+	wg := sync.WaitGroup{}
+	wg.Add(2)
+	var getUsersErr error
 	go func() {
-		defer close(ch)
-		users, err := db.GetUserNames(ctx)
-		ch <- data{
-			value: users,
-			err:   err,
-		}
+		defer wg.Done()
+		users, getUsersErr = db.GetUserNames(ctx)
 	}()
-	return ch
-}
-
-func (db *DatabaseContext) getRoleIDsInBackground(ctx context.Context) <-chan data {
-	ch := make(chan data, 1)
+	var getRolesErr error
 	go func() {
-		defer close(ch)
-		roles, err := db.getRoleIDsUsingIndex(ctx, true)
-		ch <- data{
-			value: roles,
-			err:   err,
-		}
+		defer wg.Done()
+		includeDeleted := true
+		roles, getRolesErr = db.getRoleIDsUsingIndex(ctx, includeDeleted)
 	}()
-	return ch
+	wg.Wait()
+	if getUsersErr != nil {
+		return nil, nil, getUsersErr
+	}
+	if getRolesErr != nil {
+		return nil, nil, getRolesErr
+	}
+	return users, roles, err
 }
 
 // Returns the Names of all users
@@ -1337,6 +1304,7 @@ outerLoop:
 	return users, nil
 }
 
+// getRoleIDsUsingIndex returns names of all roles. includDeleted toggles whether or not to include deleted roles.
 func (db *DatabaseContext) getRoleIDsUsingIndex(ctx context.Context, includeDeleted bool) (roles []string, err error) {
 
 	dbRoleIDPrefix := db.MetadataKeys.RoleKeyPrefix()

db/database_test.go

@@ -431,30 +431,6 @@ func TestGetRemovedAsUser(t *testing.T) {
 	assertHTTPError(t, err, 404)
 }
 
-func TestIsServerless(t *testing.T) {
-	testCases := []struct {
-		title      string
-		serverless bool
-	}{
-		{
-			serverless: true,
-		},
-		{
-			serverless: false,
-		},
-	}
-
-	for _, testCase := range testCases {
-		t.Run(fmt.Sprintf("TestIsServerless with Serverless=%t", testCase.serverless), func(t *testing.T) {
-			db, ctx := setupTestDB(t)
-			defer db.Close(ctx)
-
-			db.Options.Serverless = testCase.serverless
-			assert.Equal(t, testCase.serverless, db.IsServerless())
-		})
-	}
-}
-
 // Test removal handling for unavailable multi-channel revisions.
 func TestGetRemovalMultiChannel(t *testing.T) {
 	db, ctx := setupTestDB(t)

db/indexes.go

@@ -141,8 +141,8 @@ var (
 		IndexChannels:   IdxFlagIndexTombstones,
 		IndexAllDocs:    IdxFlagIndexTombstones,
 		IndexSyncDocs:   IdxFlagMetadataOnly | IdxFlagPrincipalDocsOnly,
-		IndexUser:       IdxFlagMetadataOnly,
-		IndexRole:       IdxFlagMetadataOnly,
+		IndexUser:       IdxFlagMetadataOnly | IdxFlagPrincipalDocsOnly,
+		IndexRole:       IdxFlagMetadataOnly | IdxFlagPrincipalDocsOnly,
 		IndexTombstones: IdxFlagXattrOnly | IdxFlagIndexTombstones,
 	}
 
@@ -261,7 +261,7 @@ func (i *SGIndex) isXattrOnly() bool {
 	return i.flags&IdxFlagXattrOnly != 0
 }
 
-// shouldCreate returns if given index should be created based on Serverless mode
+// shouldCreate returns if given index should be created.
 func (i *SGIndex) shouldCreate(options InitializeIndexOptions) bool {
 	if options.LegacySyncDocsIndex {
 		if i.creationMode == SeparatePrincipalIndexes {

db/indexes_test.go

@@ -232,6 +232,7 @@ func TestRemoveIndexesUseViewsTrueAndFalse(t *testing.T) {
 			NumReplicas:         0,
 			LegacySyncDocsIndex: db.UseLegacySyncDocsIndex(),
 			UseXattrs:           base.TestUseXattrs(),
+			NumPartitions:       DefaultNumIndexPartitions,
 		}
 		if db.OnlyDefaultCollection() {
 			options.MetadataIndexes = IndexesAll

db/indextest/util.go

@@ -24,7 +24,6 @@ func getDatabaseContextOptions(useLegacySyncDocsIndex bool) db.DatabaseContextOp
 	return db.DatabaseContextOptions{
 		CacheOptions:           &defaultCacheOptions,
 		UseLegacySyncDocsIndex: useLegacySyncDocsIndex,
-		Serverless:             !useLegacySyncDocsIndex, // after CBG-4614, this flag will not be used
 	}
 }
 

db/query.go

@@ -599,7 +599,7 @@ func (context *DatabaseContext) QueryUsers(ctx context.Context, startKey string,
 // query is covering.
 func (context *DatabaseContext) BuildUsersQuery(startKey string, limit int) (string, map[string]interface{}) {
 	var queryStatement string
-	if context.IsServerless() {
+	if !context.UseLegacySyncDocsIndex() {
 		queryStatement = replaceIndexTokensQuery(QueryUsers.statement, sgIndexes[IndexUser], context.UseXattrs(), context.numIndexPartitions())
 	} else {
 		queryStatement = replaceIndexTokensQuery(QueryUsersUsingSyncDocsIdx.statement, sgIndexes[IndexSyncDocs], context.UseXattrs(), context.numIndexPartitions())
@@ -643,7 +643,7 @@ func (context *DatabaseContext) QueryRoles(ctx context.Context, startKey string,
 func (context *DatabaseContext) BuildRolesQuery(startKey string, limit int) (string, map[string]interface{}) {
 
 	var queryStatement string
-	if context.IsServerless() {
+	if !context.UseLegacySyncDocsIndex() {
 		queryStatement = replaceIndexTokensQuery(QueryRolesExcludeDeletedUsingRoleIdx.statement, sgIndexes[IndexRole], context.UseXattrs(), context.numIndexPartitions())
 	} else {
 		queryStatement = replaceIndexTokensQuery(QueryRolesExcludeDeleted.statement, sgIndexes[IndexSyncDocs], context.UseXattrs(), context.numIndexPartitions())
@@ -668,7 +668,7 @@ func (context *DatabaseContext) QueryAllRoles(ctx context.Context, startKey stri
 	}
 
 	var queryStatement string
-	if context.IsServerless() {
+	if !context.UseLegacySyncDocsIndex() {
 		queryStatement = replaceIndexTokensQuery(QueryAllRolesUsingRoleIdx.statement, sgIndexes[IndexRole], context.UseXattrs(), context.numIndexPartitions())
 	} else {
 		queryStatement = replaceIndexTokensQuery(QueryAllRolesUsingSyncDocsIdx.statement, sgIndexes[IndexSyncDocs], context.UseXattrs(), context.numIndexPartitions())

db/util_testing.go

@@ -375,7 +375,7 @@ var viewsAndGSIBucketInit base.TBPBucketInitFunc = func(ctx context.Context, b b
 			UseXattrs:                  base.TestUseXattrs(),
 			NumReplicas:                0,
 			WaitForIndexesOnlineOption: base.WaitForIndexesDefault,
-			LegacySyncDocsIndex:        true, // Change in CBG-4614
+			LegacySyncDocsIndex:        false,
 			MetadataIndexes:            IndexesWithoutMetadata,
 			NumPartitions:              DefaultNumIndexPartitions,
 		}

rest/database_init_manager_test.go

@@ -22,6 +22,8 @@ import (
 	"github.com/stretchr/testify/require"
 )
 
+const testUseLegacySyncDocsIndex = false
+
 func TestDatabaseInitManager(t *testing.T) {
 	RequireN1QLIndexes(t)
 
@@ -45,9 +47,8 @@ func TestDatabaseInitManager(t *testing.T) {
 	// Drop indexes
 	dropAllNonPrimaryIndexes(t, tb.GetSingleDataStore())
 
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 	// Async index creation
-	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	select {
@@ -111,16 +112,15 @@ func TestDatabaseInitConfigChangeSameCollections(t *testing.T) {
 	dbConfig := makeDbConfig(tb.GetName(), dbName, collection1and2ScopesConfig)
 	require.NoError(t, dbConfig.setup(ctx, dbName, sc.Config.Bootstrap, nil, nil, false))
 
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 	// Start first async index creation, blocks after first collection
-	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Wait for first collection to be initialized
 	WaitForChannel(t, singleCollectionInitChannel, "first collection init")
 
 	// Make a duplicate call to initialize database, should reuse the existing agent
-	duplicateDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	duplicateDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Unblock collection callback to process all remaining collections
@@ -137,7 +137,7 @@ func TestDatabaseInitConfigChangeSameCollections(t *testing.T) {
 	waitForWorkerDone(t, initMgr, "dbName")
 
 	// Rerun init, should start a new worker for the database and re-verify init for each collection
-	rerunDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	rerunDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 	WaitForChannel(t, rerunDoneChan, "repeated init done chan")
 	totalCount = atomic.LoadInt64(&collectionCount)
@@ -201,9 +201,8 @@ func TestDatabaseInitConfigChangeDifferentCollections(t *testing.T) {
 	dbConfig := makeDbConfig(tb.GetName(), dbName, collection1and2ScopesConfig)
 	require.NoError(t, dbConfig.setup(ctx, dbName, sc.Config.Bootstrap, nil, nil, false))
 
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 	// Start first async index creation, should block after first collection
-	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Wait for first collection to be initialized
@@ -212,7 +211,7 @@ func TestDatabaseInitConfigChangeDifferentCollections(t *testing.T) {
 	// Make a call to initialize database for the same db name, different collections
 	modifiedDbConfig := makeDbConfig(tb.GetName(), dbName, collection1and3ScopesConfig)
 	require.NoError(t, modifiedDbConfig.setup(ctx, dbName, sc.Config.Bootstrap, nil, nil, false))
-	modifiedDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, modifiedDbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	modifiedDoneChan, err := initMgr.InitializeDatabase(ctx, sc.Config, modifiedDbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Unblock the first InitializeDatabase, should cancel
@@ -292,16 +291,15 @@ func TestDatabaseInitConcurrentDatabasesSameBucket(t *testing.T) {
 	db2Config := makeDbConfig(tb.GetName(), db2Name, collection3ScopesConfig)
 	require.NoError(t, db2Config.setup(ctx, db2Name, sc.Config.Bootstrap, nil, nil, false))
 
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 	// Start first async index creation, should block after first collection
-	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, db1Config.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, db1Config.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Wait for first collection to be initialized
 	WaitForChannel(t, firstCollectionInitChannel, "first collection init")
 
 	// Start second async index creation for db2 while first is still running
-	doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, db2Config.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, db2Config.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Unblock the first InitializeDatabase, should cancel
@@ -386,16 +384,15 @@ func TestDatabaseInitConcurrentDatabasesDifferentBuckets(t *testing.T) {
 	db2Config := makeDbConfig(tb2.GetName(), db2Name, collection1and2ScopesConfig)
 	require.NoError(t, db2Config.setup(ctx, db2Name, sc.Config.Bootstrap, nil, nil, false))
 
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 	// Start first async index creation, should block after first collection
-	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, db1Config.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, db1Config.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Wait for first collection to be initialized
 	WaitForChannel(t, firstCollectionInitChannel, "first collection init")
 
 	// Start second async index creation for db2 while first is still running
-	doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, db2Config.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, db2Config.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	// Unblock the first InitializeDatabase, should cancel
@@ -456,7 +453,6 @@ func TestDatabaseInitTeardownTiming(t *testing.T) {
 	dbName := "dbName"
 	dbConfig := makeDbConfig(tb.GetName(), dbName, collection1and2ScopesConfig)
 	require.NoError(t, dbConfig.setup(ctx, dbName, sc.Config.Bootstrap, nil, nil, false))
-	useLegacySyncDocsIndex := true // flip in CBG-4615
 
 	wg := &sync.WaitGroup{}
 	wg.Add(1)
@@ -467,7 +463,7 @@ func TestDatabaseInitTeardownTiming(t *testing.T) {
 		if currentCount == 0 {
 			defer wg.Done()
 			log.Printf("invoking InitializeDatabase again during teardown")
-			doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+			doneChan2, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 			require.NoError(t, err)
 			WaitForChannel(t, doneChan2, "done chan 2")
 		}
@@ -476,7 +472,7 @@ func TestDatabaseInitTeardownTiming(t *testing.T) {
 	}
 
 	// Start first async index creation, should block after first collection
-	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), useLegacySyncDocsIndex)
+	doneChan1, err := initMgr.InitializeDatabase(ctx, sc.Config, dbConfig.ToDatabaseConfig(), testUseLegacySyncDocsIndex)
 	require.NoError(t, err)
 
 	WaitForChannel(t, doneChan1, "done chan 1")

rest/indextest/index_test.go

@@ -62,19 +62,33 @@ func TestSyncGatewayStartupIndexes(t *testing.T) {
 		// use example indexes to make sure metadata and non metadata are created
 		indexSyncDocs := "sg_syncDocs"
 		indexAccess := "sg_access"
+		indexRoles := "sg_roles"
+		indexUsers := "sg_users"
 		if base.TestUseXattrs() {
 			indexSyncDocs += "_x1"
 			indexAccess += "_x1"
+			indexRoles += "_x1"
+			indexUsers += "_x1"
 		} else {
 			indexSyncDocs += "_1"
 			indexAccess += "_1"
+			indexRoles += "_1"
+			indexUsers += "_1"
 		}
 		metadataCollection, err := base.AsCollection(bucket.DefaultDataStore())
 		require.NoError(t, err)
 		indexNames, err := metadataCollection.GetIndexes()
 		require.NoError(t, err)
 
-		require.Contains(t, indexNames, indexSyncDocs)
+		if rt.GetDatabase().UseLegacySyncDocsIndex() {
+			require.Contains(t, indexNames, indexSyncDocs)
+			require.NotContains(t, indexNames, indexRoles)
+			require.NotContains(t, indexNames, indexUsers)
+		} else {
+			require.NotContains(t, indexNames, indexSyncDocs)
+			require.Contains(t, indexNames, indexRoles)
+			require.Contains(t, indexNames, indexUsers)
+		}
 
 		if base.TestsUseNamedCollections() {
 			require.NotContains(t, indexNames, indexAccess)