From d3353e38f30c188be66124693c6839944ab325d3 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Thu, 24 Sep 2026 10:49:18 -0400 Subject: [PATCH 1/3] CBG-5731 Finish a database delete when another node already deleted it When DELETE /db/ runs on a node after a peer deleted the database, the node now removes the database locally and returns 200 instead of 500. If the config doc is already gone when the node deletes it, the node still finalizes the registry, so no delete stays marked in progress. Registry finalize now removes the entry only if it is still the delete that this node marked. Before, a peer could recreate the database after the config doc was deleted, and finalize removed the new registry entry. Add LeakyBootstrapConnection with hooks around DeleteMetadataDocument, so that tests can simulate a peer that writes during a delete. Co-Authored-By: Claude Opus 5.5 (1M context) --- base/leaky_bootstrap_connection.go | 47 ++++ rest/admin_api.go | 3 +- rest/config_manager.go | 19 +- rest/persistent_config_test.go | 241 ++++++++++++++++++ rest/server_context.go | 8 +- rest/utilities_testing.go | 5 + rest/utilities_testing_rest_tester_cluster.go | 11 +- 7 files changed, 325 insertions(+), 9 deletions(-) create mode 100644 base/leaky_bootstrap_connection.go diff --git a/base/leaky_bootstrap_connection.go b/base/leaky_bootstrap_connection.go new file mode 100644 index 0000000000..abeec9fcf7 --- /dev/null +++ b/base/leaky_bootstrap_connection.go @@ -0,0 +1,47 @@ +/* +Copyright 2026-Present Couchbase, Inc. +Use of this software is governed by the Business Source License included in +the file licenses/BSL-Couchbase.txt. As of the Change Date specified in that +file, in accordance with the Business Source License, use of this software will +be governed by the Apache License, Version 2.0, included in the file +licenses/APL2.txt. +*/ + +package base + +import "context" + +var _ BootstrapConnection = &LeakyBootstrapConnection{} + +// LeakyBootstrapConnection is a wrapper around a BootstrapConnection to support forced errors. For testing use only. +type LeakyBootstrapConnection struct { + BootstrapConnection + config LeakyBootstrapConnectionConfig +} + +// LeakyBootstrapConnectionConfig configures the hooks of a LeakyBootstrapConnection. +type LeakyBootstrapConnectionConfig struct { + // BeforeDeleteMetadataDocument runs before DeleteMetadataDocument, to simulate a concurrent writer. + BeforeDeleteMetadataDocument func(bucket, key string) + // AfterDeleteMetadataDocument runs after DeleteMetadataDocument, to simulate a concurrent writer. + AfterDeleteMetadataDocument func(bucket, key string) +} + +// NewLeakyBootstrapConnection creates a wrapper around a BootstrapConnection to support forced errors. +func NewLeakyBootstrapConnection(conn BootstrapConnection, config LeakyBootstrapConnectionConfig) *LeakyBootstrapConnection { + return &LeakyBootstrapConnection{ + BootstrapConnection: conn, + config: config, + } +} + +func (c *LeakyBootstrapConnection) DeleteMetadataDocument(ctx context.Context, bucket, key string, cas uint64) error { + if c.config.BeforeDeleteMetadataDocument != nil { + c.config.BeforeDeleteMetadataDocument(bucket, key) + } + err := c.BootstrapConnection.DeleteMetadataDocument(ctx, bucket, key, cas) + if c.config.AfterDeleteMetadataDocument != nil { + c.config.AfterDeleteMetadataDocument(bucket, key) + } + return err +} diff --git a/rest/admin_api.go b/rest/admin_api.go index b063e65daa..cc13935a60 100644 --- a/rest/admin_api.go +++ b/rest/admin_api.go @@ -1624,7 +1624,8 @@ func (h *handler) handleDeleteDB() error { if h.server.persistentConfig { bucket, _ = h.server.bucketNameFromDbName(h.ctx(), dbName) err := h.server.BootstrapContext.DeleteConfig(h.ctx(), bucket, h.server.Config.Bootstrap.ConfigGroupID, dbName) - if err != nil { + // A not found error means another node removed the database from the registry before this node's config poller removed it. + if err != nil && !base.IsDocNotFoundError(err) { return base.HTTPErrorf(http.StatusInternalServerError, "couldn't remove database %q from bucket %q: %s", base.MD(dbName), base.MD(bucket), err.Error()) } h.server.RemoveDatabase(h.ctx(), dbName, fmt.Sprintf("called from %s", h.rq.URL)) // unhandled 404 to allow broken config deletion (CBG-2420) diff --git a/rest/config_manager.go b/rest/config_manager.go index 32ca601f87..b67476b404 100644 --- a/rest/config_manager.go +++ b/rest/config_manager.go @@ -281,6 +281,7 @@ func (b *bootstrapContext) UpdateConfig(ctx context.Context, bucketName, groupID // 4. Update the registry to remove the database definition altogether func (b *bootstrapContext) DeleteConfig(ctx context.Context, bucketName, groupID, dbName string) (err error) { var existingCas uint64 + var existingVersion string err, _ = base.RetryLoopWithOptions(ctx, "DeleteConfig", func(retryState base.RetryState) (bool, error, any) { base.InfofCtx(ctx, base.KeyConfig, "DeleteConfig starting (attempt %d/%d)", retryState.Attempt, configUpdateMaxRetryAttempts) // Step 1. Fetch registry and databases - enforces registry/config synchronization @@ -293,6 +294,7 @@ func (b *bootstrapContext) DeleteConfig(ctx context.Context, bucketName, groupID return false, base.ErrNotFound, nil } existingCas = existingConfig.cfgCas + existingVersion = existingConfig.Version // Step 2. Update registry, mark database deleted in registry if err := registry.deleteDatabase(groupID, dbName); err != nil { @@ -323,11 +325,15 @@ func (b *bootstrapContext) DeleteConfig(ctx context.Context, bucketName, groupID } err = b.Connection.DeleteMetadataDocument(ctx, bucketName, PersistentConfigKey(ctx, groupID, dbName), existingCas) - if err != nil { + if base.IsDocNotFoundError(err) { + // A peer deleted the config doc, but may not have removed the registry entry, so finalize anyway. + base.InfofCtx(ctx, base.KeyConfig, "Database config was already deleted, finalizing registry") + } else if err != nil { base.InfofCtx(ctx, base.KeyConfig, "Delete for database config returned error %v", err) return err + } else { + base.DebugfCtx(ctx, base.KeyConfig, "Delete for database config was successful") } - base.DebugfCtx(ctx, base.KeyConfig, "Delete for database config was successful") // Step 3. After config is successfully deleted, finalize the delete by removing the previous // version from the registry. Re-read + CAS retry tolerates concurrent registry writers @@ -337,10 +343,17 @@ func (b *bootstrapContext) DeleteConfig(ctx context.Context, bucketName, groupID if err != nil { return false, base.RedactErrorf("Error fetching registry to finalize delete of config group: %s, database: %s: %w", base.MD(groupID), base.MD(dbName), err), nil } - if !registry.removeDatabase(groupID, dbName) { + registryDb, found := registry.getRegistryDatabase(groupID, dbName) + if !found { base.InfofCtx(ctx, base.KeyConfig, "Database not found in registry during finalization") return false, nil, nil } + // A peer can recreate the database once the config doc is gone, so only remove the delete this node marked. + if !registryDb.IsDeleted() || registryDb.PreviousVersion == nil || registryDb.PreviousVersion.Version != existingVersion { + base.InfofCtx(ctx, base.KeyConfig, "Database was recreated in registry during finalization, leaving it in place") + return false, nil, nil + } + registry.removeDatabase(groupID, dbName) writeErr := b.setGatewayRegistry(ctx, bucketName, registry) if writeErr == nil { return false, nil, nil diff --git a/rest/persistent_config_test.go b/rest/persistent_config_test.go index 5e83d49d50..9fa5a80371 100644 --- a/rest/persistent_config_test.go +++ b/rest/persistent_config_test.go @@ -9,6 +9,7 @@ package rest import ( + "context" "encoding/json" "fmt" "net/http" @@ -1514,3 +1515,243 @@ func persistentConfigTestCases() []persistentConfigTestCase { } } } + +// createDbOnCluster creates a database on the first node and waits for every node to load it. +func createDbOnCluster(t *testing.T, rtc *RestTesterCluster, dbName string) { + RequireStatus(t, rtc.Node(0).CreateDatabase(dbName, dbConfigForTestBucket(rtc.testBucket)), http.StatusCreated) + count, err := rtc.RefreshClusterDbConfigs() + require.NoError(t, err) + require.Equal(t, rtc.NumNodes()-1, count) +} + +// TestDeleteDbConfigAlreadyDeletedByPeer deletes a database on a node after a peer deleted it, but before this node's +// config poller unloaded it. +func TestDeleteDbConfigAlreadyDeletedByPeer(t *testing.T) { + ctx := base.TestCtx(t) + rtc := NewRestTesterCluster(t, &RestTesterClusterConfig{NumNodes: 2}) + defer rtc.Close(ctx) + + const dbName = "db" + createDbOnCluster(t, rtc, dbName) + node, peer := rtc.Node(0), rtc.Node(1) + + RequireStatus(t, peer.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + require.NotNil(t, node.ServerContext().GetDatabaseConfig(dbName), "node unloaded the database before the test deleted it") + + RequireStatus(t, node.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + assert.Nil(t, node.ServerContext().GetDatabaseConfig(dbName)) +} + +// TestDeleteDbConfigDocDeletedByPeerDuringDelete deletes a database on a node while a peer deletes the same database. +// The peer finds the node's delete in progress in the registry, times out waiting for it, and completes the delete +// itself before the node removes the config doc. +func TestDeleteDbConfigDocDeletedByPeerDuringDelete(t *testing.T) { + if base.UnitTestUrlIsWalrus() { + t.Skip("TODO: CBG-4796 unskip once Rosmar returns not found instead of a CAS mismatch when removing a deleted doc") + } + const dbName = "db" + var peer *RestTester + var peerDeleteResp *TestResponse + ctx := base.TestCtx(t) + rtc := NewRestTesterCluster(t, &RestTesterClusterConfig{ + NumNodes: 2, + LeakyBootstrapConnectionConfigs: map[int]*base.LeakyBootstrapConnectionConfig{ + 0: { + BeforeDeleteMetadataDocument: func(_, _ string) { + if peer != nil && peerDeleteResp == nil { + peerDeleteResp = peer.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", "") + } + }, + }, + }, + }) + defer rtc.Close(ctx) + + createDbOnCluster(t, rtc, dbName) + node := rtc.Node(0) + peer = rtc.Node(1) + peer.ServerContext().BootstrapContext.configRetryTimeout = time.Millisecond + + RequireStatus(t, node.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + require.NotNil(t, peerDeleteResp, "peer did not delete the database during the node's delete") + RequireStatus(t, peerDeleteResp, http.StatusOK) + + rtc.ForEachNode(func(rt *RestTester) { + assert.Nil(t, rt.ServerContext().GetDatabaseConfig(dbName)) + }) + RequireStatus(t, node.CreateDatabase(dbName, dbConfigForTestBucket(rtc.testBucket)), http.StatusCreated) +} + +// TestDeleteDbConfigFinalizesRegistryAfterPeerDeletesConfigDoc deletes a database on a node while a peer removes the +// config doc but stops before it removes the database from the registry, as happens when the peer crashes. +func TestDeleteDbConfigFinalizesRegistryAfterPeerDeletesConfigDoc(t *testing.T) { + if base.UnitTestUrlIsWalrus() { + t.Skip("TODO: CBG-4796 unskip once Rosmar returns not found instead of a CAS mismatch when removing a deleted doc") + } + const dbName = "db" + var peer *RestTester + peerDeleted := false + ctx := base.TestCtx(t) + rtc := NewRestTesterCluster(t, &RestTesterClusterConfig{ + NumNodes: 2, + LeakyBootstrapConnectionConfigs: map[int]*base.LeakyBootstrapConnectionConfig{ + 0: { + BeforeDeleteMetadataDocument: func(bucket, key string) { + if peer == nil || peerDeleted { + return + } + peerDeleted = true + peerConn := peer.ServerContext().BootstrapContext.Connection + var config DatabaseConfig + cas, err := peerConn.GetMetadataDocument(ctx, bucket, key, &config) + require.NoError(t, err) + require.NoError(t, peerConn.DeleteMetadataDocument(ctx, bucket, key, cas)) + }, + }, + }, + }) + defer rtc.Close(ctx) + + createDbOnCluster(t, rtc, dbName) + node := rtc.Node(0) + peer = rtc.Node(1) + + RequireStatus(t, node.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + require.True(t, peerDeleted, "peer did not delete the config doc during the node's delete") + + sc := node.ServerContext() + registry, err := sc.BootstrapContext.getGatewayRegistry(ctx, rtc.testBucket.GetName()) + require.NoError(t, err) + registryDb, found := registry.getRegistryDatabase(sc.Config.Bootstrap.ConfigGroupID, dbName) + assert.False(t, found, "database remains in registry: %+v", registryDb) +} + +// TestDeleteDbConfigAfterInterruptedDelete deletes a database whose registry entry is marked deleted by a node that +// stopped before it removed the config doc. +func TestDeleteDbConfigAfterInterruptedDelete(t *testing.T) { + rt := NewRestTesterPersistentConfig(t) + defer rt.Close() + + ctx := rt.Context() + sc := rt.ServerContext() + bucketName := rt.Bucket().GetName() + groupID := sc.Config.Bootstrap.ConfigGroupID + sc.BootstrapContext.configRetryTimeout = time.Millisecond + + registry, err := sc.BootstrapContext.getGatewayRegistry(ctx, bucketName) + require.NoError(t, err) + require.NoError(t, registry.deleteDatabase(groupID, "db")) + require.NoError(t, sc.BootstrapContext.setGatewayRegistry(ctx, bucketName, registry)) + + RequireStatus(t, rt.SendAdminRequest(http.MethodDelete, "/db/", ""), http.StatusOK) + assert.Nil(t, sc.GetDatabaseConfig("db")) + + var config DatabaseConfig + _, err = sc.BootstrapContext.Connection.GetMetadataDocument(ctx, bucketName, PersistentConfigKey(ctx, groupID, "db"), &config) + require.True(t, base.IsDocNotFoundError(err), "expected config doc to be deleted, got %v", err) + registry, err = sc.BootstrapContext.getGatewayRegistry(ctx, bucketName) + require.NoError(t, err) + registryDb, found := registry.getRegistryDatabase(groupID, "db") + assert.False(t, found, "database remains in registry: %+v", registryDb) +} + +// startRecreateOnPeer writes the registry entry that the peer's InsertConfig writes before it writes the config doc, +// and returns the new version. +func startRecreateOnPeer(t *testing.T, ctx context.Context, peer *RestTester, bucket string, config DatabaseConfig) string { + peerCtx := peer.ServerContext().BootstrapContext + groupID := peer.ServerContext().Config.Bootstrap.ConfigGroupID + registry, existingConfig, err := peerCtx.getRegistryAndDatabase(ctx, bucket, groupID, config.Name) + require.NoError(t, err) + require.Nil(t, existingConfig) + config.Version, err = GenerateDatabaseConfigVersionID(ctx, config.Version, &config.DbConfig) + require.NoError(t, err) + _, err = registry.upsertDatabaseConfig(ctx, groupID, &config) + require.NoError(t, err) + require.NoError(t, peerCtx.setGatewayRegistry(ctx, bucket, registry)) + return config.Version +} + +// requireRegistryVersion requires the registry entry for dbName to have the given version. +func requireRegistryVersion(t *testing.T, ctx context.Context, rt *RestTester, bucket, dbName, version string) { + sc := rt.ServerContext() + registry, err := sc.BootstrapContext.getGatewayRegistry(ctx, bucket) + require.NoError(t, err) + registryDb, found := registry.getRegistryDatabase(sc.Config.Bootstrap.ConfigGroupID, dbName) + require.True(t, found, "database %s is not in the registry", dbName) + assert.Equal(t, version, registryDb.Version) +} + +// TestDeleteDbConfigKeepsDbRecreatedByPeer deletes a database on a node while a peer removes the config doc and starts +// to recreate the database, having written the new registry entry but not yet the new config doc. +func TestDeleteDbConfigKeepsDbRecreatedByPeer(t *testing.T) { + if base.UnitTestUrlIsWalrus() { + t.Skip("TODO: CBG-4796 unskip once Rosmar returns not found instead of a CAS mismatch when removing a deleted doc") + } + const dbName = "db" + var peer *RestTester + var recreatedVersion string + ctx := base.TestCtx(t) + rtc := NewRestTesterCluster(t, &RestTesterClusterConfig{ + NumNodes: 2, + LeakyBootstrapConnectionConfigs: map[int]*base.LeakyBootstrapConnectionConfig{ + 0: { + BeforeDeleteMetadataDocument: func(bucket, key string) { + if peer == nil || recreatedVersion != "" { + return + } + peerConn := peer.ServerContext().BootstrapContext.Connection + var config DatabaseConfig + cas, err := peerConn.GetMetadataDocument(ctx, bucket, key, &config) + require.NoError(t, err) + require.NoError(t, peerConn.DeleteMetadataDocument(ctx, bucket, key, cas)) + recreatedVersion = startRecreateOnPeer(t, ctx, peer, bucket, config) + }, + }, + }, + }) + defer rtc.Close(ctx) + + createDbOnCluster(t, rtc, dbName) + node := rtc.Node(0) + peer = rtc.Node(1) + + RequireStatus(t, node.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + require.NotEmpty(t, recreatedVersion, "peer did not recreate the database during the node's delete") + requireRegistryVersion(t, ctx, node, rtc.testBucket.GetName(), dbName, recreatedVersion) +} + +// TestDeleteDbConfigKeepsDbRecreatedByPeerAfterConfigDelete deletes a database on a node while a peer starts to +// recreate the database after the node removed the config doc, but before the node finalized the registry. +func TestDeleteDbConfigKeepsDbRecreatedByPeerAfterConfigDelete(t *testing.T) { + const dbName = "db" + var peer *RestTester + var deletedConfig *DatabaseConfig + var recreatedVersion string + ctx := base.TestCtx(t) + rtc := NewRestTesterCluster(t, &RestTesterClusterConfig{ + NumNodes: 2, + LeakyBootstrapConnectionConfigs: map[int]*base.LeakyBootstrapConnectionConfig{ + 0: { + AfterDeleteMetadataDocument: func(bucket, _ string) { + if deletedConfig == nil || recreatedVersion != "" { + return + } + recreatedVersion = startRecreateOnPeer(t, ctx, peer, bucket, *deletedConfig) + }, + }, + }, + }) + defer rtc.Close(ctx) + + createDbOnCluster(t, rtc, dbName) + node := rtc.Node(0) + peer = rtc.Node(1) + bucket := rtc.testBucket.GetName() + deletedConfig = &DatabaseConfig{} + _, err := node.ServerContext().BootstrapContext.Connection.GetMetadataDocument(ctx, bucket, PersistentConfigKey(ctx, node.ServerContext().Config.Bootstrap.ConfigGroupID, dbName), deletedConfig) + require.NoError(t, err) + + RequireStatus(t, node.SendAdminRequest(http.MethodDelete, "/"+dbName+"/", ""), http.StatusOK) + require.NotEmpty(t, recreatedVersion, "peer did not recreate the database during the node's delete") + requireRegistryVersion(t, ctx, node, bucket, dbName, recreatedVersion) +} diff --git a/rest/server_context.go b/rest/server_context.go index 0fb8128075..91061c7611 100644 --- a/rest/server_context.go +++ b/rest/server_context.go @@ -108,6 +108,8 @@ type ServerContext struct { RuntimeStatus *RuntimeStatus // Cached runtime environment info (GOMEMLIMIT, GOMAXPROCS, cgroup), computed once at startup ClusterCompat *clusterCompatManager // Tracks cluster-wide minimum SG version for compat gating connectToBucketFn db.OpenBucketFn // supply a custom function for buckets, used for testing only + // leakyBootstrapConnectionConfig wraps the bootstrap connection in a LeakyBootstrapConnection, used for testing only + leakyBootstrapConnectionConfig *base.LeakyBootstrapConnectionConfig } type ActiveReplicationsCounter struct { @@ -2276,7 +2278,11 @@ func (sc *ServerContext) initializeBootstrapConnection(ctx context.Context) erro return err } - sc.BootstrapContext.Connection = couchbaseCluster + if sc.leakyBootstrapConnectionConfig != nil { + sc.BootstrapContext.Connection = base.NewLeakyBootstrapConnection(couchbaseCluster, *sc.leakyBootstrapConnectionConfig) + } else { + sc.BootstrapContext.Connection = couchbaseCluster + } // Check for v3.0 persisted configs, migrate to registry format if found err = sc.migrateV30Configs(ctx) diff --git a/rest/utilities_testing.go b/rest/utilities_testing.go index ce46f57573..abd3b26506 100644 --- a/rest/utilities_testing.go +++ b/rest/utilities_testing.go @@ -80,6 +80,7 @@ type RestTesterConfig struct { maxConcurrentRevs *int UseXattrConfig bool UseSystemScopeMetadataCollection *bool + LeakyBootstrapConnectionConfig *base.LeakyBootstrapConnectionConfig // Set to wrap the bootstrap connection in a LeakyBootstrapConnection. Requires PersistentConfig. } type collectionConfiguration uint8 @@ -180,6 +181,9 @@ func newRestTester(tb testing.TB, restConfig *RestTesterConfig, collectionConfig } else { rt.RestTesterConfig = &RestTesterConfig{} } + if rt.RestTesterConfig.LeakyBootstrapConnectionConfig != nil && !rt.RestTesterConfig.PersistentConfig { + require.FailNow(tb, "LeakyBootstrapConnectionConfig requires PersistentConfig") + } rt.RestTesterConfig.collectionConfig = collectionConfig rt.RestTesterConfig.numCollections = numCollections rt.RestTesterConfig.useTLSServer = base.ServerIsTLS(sgtest.UnitTestUrl()) @@ -315,6 +319,7 @@ func (rt *RestTester) Bucket() base.Bucket { } } rt.RestTesterServerContext.allowScopesInPersistentConfig = true + rt.RestTesterServerContext.leakyBootstrapConnectionConfig = rt.RestTesterConfig.LeakyBootstrapConnectionConfig if rt.RestTesterConfig.nodeClusterCompatVersion != nil { rt.RestTesterServerContext.BootstrapContext.clusterCompatVersion = *rt.RestTesterConfig.nodeClusterCompatVersion } diff --git a/rest/utilities_testing_rest_tester_cluster.go b/rest/utilities_testing_rest_tester_cluster.go index 0aa91da6b2..eff6c04f74 100644 --- a/rest/utilities_testing_rest_tester_cluster.go +++ b/rest/utilities_testing_rest_tester_cluster.go @@ -109,6 +109,8 @@ func (rtc *RestTesterCluster) Close(ctx context.Context) { type RestTesterClusterConfig struct { NumNodes uint8 // Number of RestTester objects to create MutateStartupConfig func(*StartupConfig) // Passes this option to the RestTesterConfig for each RestTester + // LeakyBootstrapConnectionConfigs maps a node index to the LeakyBootstrapConnectionConfig for that node + LeakyBootstrapConnectionConfigs map[int]*base.LeakyBootstrapConnectionConfig } func defaultRestTesterClusterConfig() *RestTesterClusterConfig { @@ -135,10 +137,11 @@ func NewRestTesterCluster(t *testing.T, config *RestTesterClusterConfig) *RestTe wg.Go(func() { // RestTesterConfig is mutated by NewRestTester, make a new instance in each loop rtConfig := &RestTesterConfig{ - GroupID: &groupID, - PersistentConfig: true, - CustomTestBucket: tb.NoCloseClone(), - MutateStartupConfig: config.MutateStartupConfig, + GroupID: &groupID, + PersistentConfig: true, + CustomTestBucket: tb.NoCloseClone(), + MutateStartupConfig: config.MutateStartupConfig, + LeakyBootstrapConnectionConfig: config.LeakyBootstrapConnectionConfigs[int(i)], } rt := NewRestTester(t, rtConfig) // initialize the RestTester before we attempt to use it From f8da1f0dce634dec5b2428f5d76363852d0a004a Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Mon, 28 Sep 2026 21:21:01 -0400 Subject: [PATCH 2/3] Don't shadow errors so the error is passed down the call stack --- rest/config_manager.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/rest/config_manager.go b/rest/config_manager.go index b67476b404..6f2da7c0b8 100644 --- a/rest/config_manager.go +++ b/rest/config_manager.go @@ -1097,13 +1097,17 @@ func (b *bootstrapContext) getRegistryAndDatabase(ctx context.Context, bucketNam } } else if registryDb.PreviousVersion != nil { // Previous Version without current version represents in-progress delete. Wait for delete to complete - err := b.waitForConfigDelete(ctx, bucketName, groupID, dbName, registryDb.PreviousVersion.Version, registry) - if err == base.ErrConfigRegistryReloadRequired { + err = b.waitForConfigDelete(ctx, bucketName, groupID, dbName, registryDb.PreviousVersion.Version, registry) + if errors.Is(err, base.ErrConfigRegistryReloadRequired) { // ReloadRegistry is returned by waitForConfigDelete immediately if the config exists but the // version does not match the previous version. Indicates a concurrent author has recreated the // database - continue to reload the registry. continue } + // This node finished the interrupted delete, so the database no longer exists. + if errors.Is(err, base.ErrConfigRegistryRollback) { + return registry, nil, nil + } } // Otherwise we're done (either success, or return unrecoverable error) From 95076a0b5ef9e85300bf10bd295ab589227ad109 Mon Sep 17 00:00:00 2001 From: Tor Colvin Date: Mon, 28 Sep 2026 21:23:34 -0400 Subject: [PATCH 3/3] PR comments about test code --- rest/utilities_testing.go | 10 ---------- rest/utilities_testing_rest_tester_cluster.go | 13 +++++++------ 2 files changed, 7 insertions(+), 16 deletions(-) diff --git a/rest/utilities_testing.go b/rest/utilities_testing.go index abd3b26506..4bbd03f252 100644 --- a/rest/utilities_testing.go +++ b/rest/utilities_testing.go @@ -1166,16 +1166,6 @@ func (rt *RestTester) GetRawDoc(key string) RawDocResponse { return rawResponse } -// ReplacePerBucketCredentials replaces buckets defined on StartupConfig.BucketCredentials then recreates the couchbase -// cluster to pick up the changes -func (rt *RestTester) ReplacePerBucketCredentials(config base.PerBucketCredentialsConfig) { - rt.ServerContext().Config.BucketCredentials = config - // Update the CouchbaseCluster to include the new bucket credentials - couchbaseCluster, err := CreateBootstrapConnectionFromStartupConfig(base.TestCtx(rt.TB()), rt.ServerContext().Config, base.PerUseClusterConnections) - require.NoError(rt.TB(), err) - rt.ServerContext().BootstrapContext.Connection = couchbaseCluster -} - // Context returns a context for a rest tester with server and database log context, if available an unambiguous. func (rt *RestTester) Context() context.Context { ctx := base.TestCtx(rt.TB()) diff --git a/rest/utilities_testing_rest_tester_cluster.go b/rest/utilities_testing_rest_tester_cluster.go index eff6c04f74..fb35aeb946 100644 --- a/rest/utilities_testing_rest_tester_cluster.go +++ b/rest/utilities_testing_rest_tester_cluster.go @@ -79,10 +79,11 @@ func (rtc *RestTesterCluster) AddNode() *RestTester { expectedDbNames := nodes[0].ServerContext().AllDatabaseNames() rtConfig := &RestTesterConfig{ - GroupID: &rtc.groupID, - PersistentConfig: true, - CustomTestBucket: rtc.testBucket.NoCloseClone(), - MutateStartupConfig: rtc.config.MutateStartupConfig, + GroupID: &rtc.groupID, + PersistentConfig: true, + CustomTestBucket: rtc.testBucket.NoCloseClone(), + MutateStartupConfig: rtc.config.MutateStartupConfig, + LeakyBootstrapConnectionConfig: rtc.config.LeakyBootstrapConnectionConfigs[len(nodes)], } rt := NewRestTester(rtc.t, rtConfig) sc := rt.ServerContext() @@ -92,8 +93,8 @@ func (rtc *RestTesterCluster) AddNode() *RestTester { require.ElementsMatch(rtc.t, expectedDbNames, sc.AllDatabaseNames(), "new node did not discover the same databases as the rest of the cluster") rtc.restTestersLock.Lock() + defer rtc.restTestersLock.Unlock() rtc._restTesters = append(rtc._restTesters, rt) - rtc.restTestersLock.Unlock() return rt } @@ -109,7 +110,7 @@ func (rtc *RestTesterCluster) Close(ctx context.Context) { type RestTesterClusterConfig struct { NumNodes uint8 // Number of RestTester objects to create MutateStartupConfig func(*StartupConfig) // Passes this option to the RestTesterConfig for each RestTester - // LeakyBootstrapConnectionConfigs maps a node index to the LeakyBootstrapConnectionConfig for that node + // LeakyBootstrapConnectionConfigs maps a node index to the LeakyBootstrapConnectionConfig for that node, including nodes started by AddNode LeakyBootstrapConnectionConfigs map[int]*base.LeakyBootstrapConnectionConfig }