CBG-5357: removal of fast fail retry by default (#8308)

Author: gregns1

base/bootstrap.go

@@ -13,6 +13,7 @@ import (
 	"encoding/json"
 	"errors"
 	"fmt"
+	"net/http"
 	"reflect"
 	"sort"
 	"sync"
@@ -91,13 +92,14 @@ type BootstrapConnection interface {
 
 // CouchbaseClusterSpec define how to make a connection to Couchbase Server
 type CouchbaseClusterSpec struct {
-	Server        string // connection string to connect to the Couchbase cluster
-	Username      string // RBAC username to authenticate with the cluster
-	Password      string // RBAC password to authenticate with the cluster
-	X509Certpath  string // X.509 cert path to authenticate with the cluster
-	X509Keypath   string // X.509 key path to authenticate with the cluster
-	CACertpath    string // CA cert path to use for TLS connections
-	TLSSkipVerify bool   // If true, do not validate TLS certificate
+	Server               string // connection string to connect to the Couchbase cluster
+	Username             string // RBAC username to authenticate with the cluster
+	Password             string // RBAC password to authenticate with the cluster
+	X509Certpath         string // X.509 cert path to authenticate with the cluster
+	X509Keypath          string // X.509 key path to authenticate with the cluster
+	CACertpath           string // CA cert path to use for TLS connections
+	TLSSkipVerify        bool   // If true, do not validate TLS certificate
+	UseGOCBFastFailRetry bool   // When true, readiness checks fail fast instead of using the best-effort retry strategy
 }
 
 // CouchbaseCluster is a GoCBv2 implementation of BootstrapConnection
@@ -117,6 +119,7 @@ type CouchbaseCluster struct {
 	// Resolved lazily on first interaction (or eagerly via SetBucketBootstrapTargetHint). Values
 	// are bucketBootstrapTarget; absence of an entry means "fall back to the connection-wide flag."
 	bucketBootstrapTargets sync.Map
+	useGOCBFastFailRetry   bool // When true, readiness checks fail fast instead of using the best-effort retry strategy
 }
 
 // bucketBootstrapTarget records where bootstrap docs (registry, dbconfig, cbgt cfg) live for a
@@ -255,6 +258,7 @@ func NewCouchbaseCluster(ctx context.Context, clusterSpec CouchbaseClusterSpec,
 		clusterOptions:              clusterOptions,
 		bucketConnectionMode:        bucketMode,
 		useSystemMetadataCollection: useSystemMetadataCollection,
+		useGOCBFastFailRetry:        clusterSpec.UseGOCBFastFailRetry,
 	}
 
 	if bucketMode == CachedClusterConnections {
@@ -287,7 +291,7 @@ func (cc *CouchbaseCluster) connect(auth *gocb.Authenticator) (*gocb.Cluster, er
 	err = cluster.WaitUntilReady(time.Second*10, &gocb.WaitUntilReadyOptions{
 		DesiredState:  gocb.ClusterStateOnline,
 		ServiceTypes:  []gocb.ServiceType{gocb.ServiceTypeManagement},
-		RetryStrategy: &goCBv2FailFastRetryStrategy{},
+		RetryStrategy: gocbRetryStrategy(cc.useGOCBFastFailRetry),
 	})
 	if err != nil {
 		_ = cluster.Close(nil)
@@ -1049,7 +1053,7 @@ func (cc *CouchbaseCluster) connectToBucket(ctx context.Context, bucketName stri
 	b = connection.Bucket(bucketName)
 	err = b.WaitUntilReady(time.Second*10, &gocb.WaitUntilReadyOptions{
 		DesiredState:  gocb.ClusterStateOnline,
-		RetryStrategy: &goCBv2FailFastRetryStrategy{},
+		RetryStrategy: gocbRetryStrategy(cc.useGOCBFastFailRetry),
 		ServiceTypes:  []gocb.ServiceType{gocb.ServiceTypeKeyValue},
 	})
 	if err != nil {
@@ -1059,6 +1063,13 @@ func (cc *CouchbaseCluster) connectToBucket(ctx context.Context, bucketName stri
 			return nil, nil, ErrAuthError
 		}
 
+		// In best-effort retry mode a missing/unreachable bucket surfaces as a timeout rather than an
+		// auth failure. Classify it as a connection error so callers translate it to a 502 instead of a
+		// generic 500, mirroring the per-database connection path (see db.connectToBucketErrorHandling).
+		if !cc.useGOCBFastFailRetry {
+			return nil, nil, HTTPErrorf(http.StatusBadGateway,
+				"Unable to connect to Couchbase Server. Please ensure it is running and reachable, and that bucket %q exists. Error: %s", MD(bucketName).Redact(), err)
+		}
 		return nil, nil, err
 	}
 

base/bucket.go

@@ -121,6 +121,7 @@ type BucketSpec struct {
 	ViewQueryTimeoutSecs          *uint32        // the view query timeout in seconds (default: 75 seconds)
 	MaxConcurrentQueryOps         *int           // maximum number of concurrent query operations (default: DefaultMaxConcurrentQueryOps)
 	BucketOpTimeout               *time.Duration // How long bucket ops should block returning "operation timed out". If nil, uses GoCB default.  GoCB buckets only.
+	FastFailOnInitialConnection   bool           // When true, gocb cluster and bucket readiness checks fail fast instead of using the best-effort retry strategy
 }
 
 const defaultNumRetries = 10

base/bucket_gocb_test.go

@@ -2087,31 +2087,41 @@ func TestCouchbaseServerIncorrectLogin(t *testing.T) {
 	if UnitTestUrlIsWalrus() {
 		t.Skip("This test only works against Couchbase Server")
 	}
+	LongRunningTest(t)
 
 	ctx := TestCtx(t)
 	for _, tls := range []bool{true, false} {
-		t.Run(fmt.Sprintf("tls=%v", tls), func(t *testing.T) {
-			testBucket := GetTestBucket(t)
-			defer testBucket.Close(ctx)
-
-			// Override test bucket spec with invalid creds
-			testBucket.BucketSpec.Auth = TestAuthenticator{
-				Username:   "invalid_username",
-				Password:   "invalid_password",
-				BucketName: testBucket.BucketSpec.BucketName,
-			}
-			if tls {
-				testBucket.BucketSpec.Server = strings.ReplaceAll(testBucket.BucketSpec.Server, "couchbase://", "couchbases://")
-				testBucket.BucketSpec.TLSSkipVerify = true // test env isn't always using valid certs
-			} else {
-				testBucket.BucketSpec.Server = strings.ReplaceAll(testBucket.BucketSpec.Server, "couchbases://", "couchbase://")
-			}
+		for _, fastFail := range []bool{true, false} {
+			t.Run(fmt.Sprintf("tls=%v/fastFail=%v", tls, fastFail), func(t *testing.T) {
+				testBucket := GetTestBucket(t)
+				defer testBucket.Close(ctx)
+
+				// Override test bucket spec with invalid creds
+				testBucket.BucketSpec.Auth = TestAuthenticator{
+					Username:   "invalid_username",
+					Password:   "invalid_password",
+					BucketName: testBucket.BucketSpec.BucketName,
+				}
+				testBucket.BucketSpec.FastFailOnInitialConnection = fastFail
+				if tls {
+					testBucket.BucketSpec.Server = strings.ReplaceAll(testBucket.BucketSpec.Server, "couchbase://", "couchbases://")
+					testBucket.BucketSpec.TLSSkipVerify = true // test env isn't always using valid certs
+				} else {
+					testBucket.BucketSpec.Server = strings.ReplaceAll(testBucket.BucketSpec.Server, "couchbases://", "couchbase://")
+				}
 
-			// Attempt to open the bucket again using invalid creds. We should expect an error.
-			bucket, err := GetBucket(TestCtx(t), testBucket.BucketSpec)
-			assert.Equal(t, ErrAuthError, err)
-			assert.Nil(t, bucket)
-		})
+				// Attempt to open the bucket again using invalid creds. We should expect an error.
+				bucket, err := GetBucket(TestCtx(t), testBucket.BucketSpec)
+				assert.Nil(t, bucket)
+				if fastFail {
+					// Fail-fast strategy surfaces the authentication failure directly.
+					assert.Equal(t, ErrAuthError, err)
+				} else {
+					// Best-effort strategy retries the auth failure until WaitUntilReady times out.
+					assert.ErrorIs(t, err, gocb.ErrUnambiguousTimeout)
+				}
+			})
+		}
 	}
 }
 

base/collection.go

@@ -77,7 +77,7 @@ func GetGoCBv2Bucket(ctx context.Context, spec BucketSpec) (*GocbV2Bucket, error
 	err = cluster.WaitUntilReady(time.Second*30, &gocb.WaitUntilReadyOptions{
 		DesiredState:  gocb.ClusterStateOnline,
 		ServiceTypes:  []gocb.ServiceType{gocb.ServiceTypeManagement},
-		RetryStrategy: &goCBv2FailFastRetryStrategy{},
+		RetryStrategy: gocbRetryStrategy(spec.FastFailOnInitialConnection),
 	})
 
 	if err != nil {
@@ -89,7 +89,7 @@ func GetGoCBv2Bucket(ctx context.Context, spec BucketSpec) (*GocbV2Bucket, error
 		return nil, err
 	}
 
-	return GetGocbV2BucketFromCluster(ctx, cluster, spec, connString, time.Second*30, true)
+	return GetGocbV2BucketFromCluster(ctx, cluster, spec, connString, time.Second*30, spec.FastFailOnInitialConnection)
 
 }
 
@@ -111,14 +111,8 @@ func GetGocbV2BucketFromCluster(ctx context.Context, cluster *gocb.Cluster, spec
 	// Connect to bucket
 	bucket := cluster.Bucket(spec.BucketName)
 
-	var retryStrategy gocb.RetryStrategy
-	if failFast {
-		retryStrategy = &goCBv2FailFastRetryStrategy{}
-	} else {
-		retryStrategy = gocb.NewBestEffortRetryStrategy(nil)
-	}
 	err := bucket.WaitUntilReady(waitUntilReady, &gocb.WaitUntilReadyOptions{
-		RetryStrategy: retryStrategy,
+		RetryStrategy: gocbRetryStrategy(failFast),
 	})
 	if err != nil {
 		_ = cluster.Close(&gocb.ClusterCloseOptions{})

base/collection_n1ql_common.go

@@ -109,9 +109,7 @@ type indexManager struct {
 }
 
 func (im *indexManager) GetAllIndexes() ([]gocb.QueryIndex, error) {
-	opts := &gocb.GetAllQueryIndexesOptions{
-		RetryStrategy: &goCBv2FailFastRetryStrategy{},
-	}
+	opts := &gocb.GetAllQueryIndexesOptions{}
 
 	if im.collection != nil {
 		return im.collection.GetAllIndexes(opts)

base/gocb_utils.go

@@ -83,6 +83,15 @@ func (rs *goCBv2FailFastRetryStrategy) RetryAfter(req gocb.RetryRequest, reason
 	return &gocb.NoRetryRetryAction{}
 }
 
+// gocbRetryStrategy returns the fail-fast retry strategy when useFailFast is true, otherwise the best-effort
+// strategy that retries until the operation's timeout. Gated by the unsupported.use_gocb_fast_fail_retry config.
+func gocbRetryStrategy(useFailFast bool) gocb.RetryStrategy {
+	if useFailFast {
+		return &goCBv2FailFastRetryStrategy{}
+	}
+	return gocb.NewBestEffortRetryStrategy(nil)
+}
+
 // GOCBCORE Utilities
 
 // CertificateAuthenticator allows for certificate auth in gocbcore

base/gocb_utils_test.go

@@ -11,6 +11,7 @@ package base
 import (
 	"testing"
 
+	"github.com/couchbase/gocb/v2"
 	"github.com/stretchr/testify/require"
 
 	"github.com/stretchr/testify/assert"
@@ -95,3 +96,8 @@ func TestGoCBCoreAuthConfigInvalidPaths(t *testing.T) {
 	_, err := GoCBCoreAuthConfig("", "", "/non/existent/cert", "/non/existent/key")
 	assert.Error(t, err)
 }
+
+func TestGoCBRetryStrategy(t *testing.T) {
+	assert.IsType(t, &goCBv2FailFastRetryStrategy{}, gocbRetryStrategy(true))
+	assert.IsType(t, gocb.NewBestEffortRetryStrategy(nil), gocbRetryStrategy(false))
+}

docs/api/components/schemas.yaml

@@ -2493,6 +2493,10 @@ Startup-config:
           description: Store database configurations in system xattrs
           type: boolean
           default: false
+        use_gocb_fast_fail_retry:
+          description: When true, errors on initial connection to Couchbase Server will fail instantaneously. Enabling this will surface authentication errors quickly, but can cause some Sync Gateway operations to shut down databases with intermittent Couchbase Server connection errors.
+          type: boolean
+          default: false
       readOnly: true
     couchbase_keepalive_interval:
       description: TCP keep-alive interval between SG and Couchbase server. This is unused.