CBG-5731 Finish a database delete when another node already deleted it

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
+}

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)

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
@@ -1084,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)

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)
+}