Merge cockroachdb#138696 cockroachdb#139040 cockroachdb#139042

138696: sql: add CHECK EXTERNAL CONNECTION command r=kev-cao a=kev-cao

This patch adds the `CHECK EXTERNAL CONNECTION` command and replaces the old `SHOW BACKUP CONNECTION` syntax.

Epic: None

Release note: None

139040: roachtest: disable shared process rebalance/by-load roachtests r=arulajmani a=kvoli

In cockroachdb#129117, shared process deployments were added to the `rebalance/by-load/*/mixed-version` roachtests. Despite multiple deflaking attempts (cockroachdb#131787, cockroachdb#133681, cockroachdb#136115, cockroachdb#136116), these continue to fail weekly. The root cause is currently unknown.

Part of: cockroachdb#139037
Release note: None

139042: roachtest: fix single node sysbench setup r=srosenberg a=DarrylWong

The single node setup was accidentally using HAProxy for the prepare step, causing it to silently fail and segfault later on. Since segfaults are an ignored error, the single node tests were silently failing.

Fixes: cockroachdb#139024
Epic: none
Release note: none

Co-authored-by: Kevin Cao <[email protected]>
Co-authored-by: Austen McClernon <[email protected]>
Co-authored-by: DarrylWong <[email protected]>

docs/generated/sql/bnf/BUILD.bazel

@@ -117,6 +117,8 @@ FILES = [
     "create_trigger",
     "create_type",
     "create_view_stmt",
+    "check_stmt",
+    "check_external_connection_stmt",
     "deallocate_stmt",
     "declare_cursor_stmt",
     "default_value_column_level",

docs/generated/sql/bnf/check_external_connection_stmt.bnf

@@ -0,0 +1,2 @@
+check_external_connection_stmt ::=
+	'CHECK' 'EXTERNAL' 'CONNECTION' string_or_placeholder opt_with_check_external_connection_options_list

docs/generated/sql/bnf/check_stmt.bnf

@@ -0,0 +1,2 @@
+check_stmt ::=
+	check_external_connection_stmt

docs/generated/sql/bnf/preparable_stmt.bnf

@@ -3,6 +3,7 @@ preparable_stmt ::=
 	| backup_stmt
 	| cancel_stmt
 	| create_stmt
+	| check_stmt
 	| delete_stmt
 	| drop_stmt
 	| explain_stmt

docs/generated/sql/bnf/stmt_block.bnf

@@ -43,6 +43,7 @@ preparable_stmt ::=
 	| backup_stmt
 	| cancel_stmt
 	| create_stmt
+	| check_stmt
 	| delete_stmt
 	| drop_stmt
 	| explain_stmt
@@ -212,6 +213,9 @@ create_stmt ::=
 	| create_logical_replication_stream_stmt
 	| create_schedule_stmt
 
+check_stmt ::=
+	check_external_connection_stmt
+
 delete_stmt ::=
 	opt_with_clause 'DELETE' opt_batch_clause 'FROM' table_expr_opt_alias_idx opt_using_clause opt_where_clause opt_sort_clause opt_limit_clause returning_clause
 
@@ -635,6 +639,9 @@ create_schedule_stmt ::=
 	create_schedule_for_changefeed_stmt
 	| create_schedule_for_backup_stmt
 
+check_external_connection_stmt ::=
+	'CHECK' 'EXTERNAL' 'CONNECTION' string_or_placeholder opt_with_check_external_connection_options_list
+
 opt_with_clause ::=
 	with_clause
 	| 
@@ -1909,6 +1916,11 @@ create_schedule_for_changefeed_stmt ::=
 create_schedule_for_backup_stmt ::=
 	'CREATE' 'SCHEDULE' schedule_label_spec 'FOR' 'BACKUP' opt_backup_targets 'INTO' string_or_placeholder_opt_list opt_with_backup_options cron_expr opt_full_backup_clause opt_with_schedule_options
 
+opt_with_check_external_connection_options_list ::=
+	'WITH' check_external_connection_options_list
+	| 'WITH' 'OPTIONS' '(' check_external_connection_options_list ')'
+	| 
+
 with_clause ::=
 	'WITH' cte_list
 	| 'WITH' 'RECURSIVE' cte_list
@@ -2728,6 +2740,9 @@ opt_full_backup_clause ::=
 	| 'FULL' 'BACKUP' 'ALWAYS'
 	| 
 
+check_external_connection_options_list ::=
+	( check_external_connection_options ) ( ( ',' check_external_connection_options ) )*
+
 cte_list ::=
 	( common_table_expr ) ( ( ',' common_table_expr ) )*
 
@@ -3365,6 +3380,11 @@ logical_replication_create_table_options ::=
 	| 'UNIDIRECTIONAL'
 	| 'BIDIRECTIONAL' 'ON' string_or_placeholder
 
+check_external_connection_options ::=
+	'TRANSFER' '=' string_or_placeholder
+	| 'TIME' '=' string_or_placeholder
+	| 'CONCURRENTLY' '=' a_expr
+
 common_table_expr ::=
 	table_alias_name opt_col_def_list_no_types 'AS' materialize_clause '(' preparable_stmt ')'
 

pkg/BUILD.bazel

@@ -1037,10 +1037,7 @@ GO_TARGETS = [
     "//pkg/cloud/amazon:amazon_test",
     "//pkg/cloud/azure:azure",
     "//pkg/cloud/azure:azure_test",
-    "//pkg/cloud/cloudcheck:cloudcheck",
     "//pkg/cloud/cloudpb:cloudpb",
-    "//pkg/cloud/cloudprivilege:cloudprivilege",
-    "//pkg/cloud/cloudprivilege:privileges",
     "//pkg/cloud/cloudtestutils:cloudtestutils",
     "//pkg/cloud/externalconn/connectionpb:connectionpb",
     "//pkg/cloud/externalconn/providers:providers",

pkg/backup/BUILD.bazel

@@ -51,9 +51,7 @@ go_library(
         "//pkg/ccl/multiregionccl",
         "//pkg/ccl/storageccl",
         "//pkg/cloud",
-        "//pkg/cloud/cloudcheck",
         "//pkg/cloud/cloudpb",
-        "//pkg/cloud/cloudprivilege",
         "//pkg/clusterversion",
         "//pkg/featureflag",
         "//pkg/jobs",

pkg/backup/backup_planning.go

@@ -14,7 +14,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/backup/backupbase"
 	"github.com/cockroachdb/cockroach/pkg/backup/backupresolver"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
-	"github.com/cockroachdb/cockroach/pkg/cloud/cloudprivilege"
 	"github.com/cockroachdb/cockroach/pkg/featureflag"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
@@ -271,7 +270,7 @@ func checkPrivilegesForBackup(
 					pgcode.InsufficientPrivilege,
 					"only users with the admin role or the BACKUP system privilege are allowed to perform full cluster backups")
 			}
-			return cloudprivilege.CheckDestinationPrivileges(ctx, p, to)
+			return sql.CheckDestinationPrivileges(ctx, p, to)
 		}
 	}
 
@@ -328,7 +327,7 @@ func checkPrivilegesForBackup(
 	// If a user has the BACKUP privilege on the target databases or tables we can
 	// move on to checking the destination URIs.
 	if hasRequiredBackupPrivileges {
-		return cloudprivilege.CheckDestinationPrivileges(ctx, p, to)
+		return sql.CheckDestinationPrivileges(ctx, p, to)
 	}
 
 	// The following checks are to maintain compatability with pre-22.2 privilege
@@ -362,7 +361,7 @@ func checkPrivilegesForBackup(
 		}
 	}
 
-	return cloudprivilege.CheckDestinationPrivileges(ctx, p, to)
+	return sql.CheckDestinationPrivileges(ctx, p, to)
 }
 
 func backupTypeCheck(

pkg/backup/restore_planning.go

@@ -21,7 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/ccl/multiregionccl"
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
-	"github.com/cockroachdb/cockroach/pkg/cloud/cloudprivilege"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/featureflag"
 	"github.com/cockroachdb/cockroach/pkg/jobs"
@@ -1436,7 +1435,7 @@ func restorePlanHook(
 func checkRestoreDestinationPrivileges(
 	ctx context.Context, p sql.PlanHookState, from []string,
 ) error {
-	return cloudprivilege.CheckDestinationPrivileges(ctx, p, from)
+	return sql.CheckDestinationPrivileges(ctx, p, from)
 }
 
 // checkRestorePrivilegesOnDatabase check that the user has adequate privileges

pkg/backup/show.go

@@ -21,8 +21,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/backup/backuputils"
 	"github.com/cockroachdb/cockroach/pkg/ccl/storageccl"
 	"github.com/cockroachdb/cockroach/pkg/cloud"
-	"github.com/cockroachdb/cockroach/pkg/cloud/cloudcheck"
-	"github.com/cockroachdb/cockroach/pkg/cloud/cloudprivilege"
 	"github.com/cockroachdb/cockroach/pkg/clusterversion"
 	"github.com/cockroachdb/cockroach/pkg/jobs/jobspb"
 	"github.com/cockroachdb/cockroach/pkg/keys"
@@ -45,7 +43,6 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/sem/tree"
 	"github.com/cockroachdb/cockroach/pkg/sql/types"
 	"github.com/cockroachdb/cockroach/pkg/util/hlc"
-	"github.com/cockroachdb/cockroach/pkg/util/humanizeutil"
 	"github.com/cockroachdb/cockroach/pkg/util/json"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
@@ -208,9 +205,6 @@ func showBackupTypeCheck(
 	); err != nil {
 		return false, nil, err
 	}
-	if backup.Details == tree.BackupConnectionTest {
-		return true, cloudcheck.Header, nil
-	}
 	infoReader := getBackupInfoReader(p, backup)
 	return true, infoReader.header(), nil
 }
@@ -225,45 +219,6 @@ func showBackupPlanHook(
 	}
 	exprEval := p.ExprEvaluator("SHOW BACKUP")
 
-	// TODO(dt): find move this to its own hook.
-	if showStmt.Details == tree.BackupConnectionTest {
-		loc, err := exprEval.String(ctx, showStmt.Path)
-		if err != nil {
-			return nil, nil, false, err
-		}
-		var params cloudcheck.Params
-		if showStmt.Options.CheckConnectionTransferSize != nil {
-			transferSizeStr, err := exprEval.String(ctx, showStmt.Options.CheckConnectionTransferSize)
-			if err != nil {
-				return nil, nil, false, err
-			}
-			parsed, err := humanizeutil.ParseBytes(transferSizeStr)
-			if err != nil {
-				return nil, nil, false, err
-			}
-			params.TransferSize = parsed
-		}
-		if showStmt.Options.CheckConnectionDuration != nil {
-			durationStr, err := exprEval.String(ctx, showStmt.Options.CheckConnectionDuration)
-			if err != nil {
-				return nil, nil, false, err
-			}
-			parsed, err := time.ParseDuration(durationStr)
-			if err != nil {
-				return nil, nil, false, err
-			}
-			params.MinDuration = parsed
-		}
-		if showStmt.Options.CheckConnectionConcurrency != nil {
-			concurrency, err := exprEval.Int(ctx, showStmt.Options.CheckConnectionConcurrency)
-			if err != nil {
-				return nil, nil, false, err
-			}
-			params.Concurrency = concurrency
-		}
-		return cloudcheck.ShowCloudStorageTestPlanHook(ctx, p, loc, params)
-	}
-
 	if showStmt.Path == nil && showStmt.InCollection != nil {
 		collection, err := exprEval.StringArray(
 			ctx, tree.Exprs(showStmt.InCollection),
@@ -319,7 +274,7 @@ func showBackupPlanHook(
 					"https://www.cockroachlabs.com/docs/stable/show-backup.html"))
 		}
 
-		if err := cloudprivilege.CheckDestinationPrivileges(ctx, p, dest); err != nil {
+		if err := sql.CheckDestinationPrivileges(ctx, p, dest); err != nil {
 			return err
 		}
 
@@ -1517,7 +1472,7 @@ func showBackupsInCollectionPlanHook(
 	ctx context.Context, collection []string, showStmt *tree.ShowBackup, p sql.PlanHookState,
 ) (sql.PlanHookRowFn, colinfo.ResultColumns, bool, error) {
 
-	if err := cloudprivilege.CheckDestinationPrivileges(ctx, p, collection); err != nil {
+	if err := sql.CheckDestinationPrivileges(ctx, p, collection); err != nil {
 		return nil, nil, false, err
 	}
 

pkg/backup/testdata/backup-restore/show_backup

@@ -63,17 +63,3 @@ query-sql regex=No\sproblems\sfound!
 SELECT * FROM [SHOW BACKUP VALIDATE FROM 'valid-22.2-with-job' IN 'nodelocal://1/'];
 ----
 true
-
-query-sql
-SELECT node, locality, ok, error, can_delete FROM [SHOW BACKUP CONNECTION 'nodelocal://1/conn-test' WITH TRANSFER='1'] ORDER BY node;
-----
-1 region=eu-central-1,availability-zone=eu-central-1 true  true
-2 region=eu-north-1,availability-zone=eu-north-1 true  true
-3 region=us-east-1,availability-zone=us-east1 true  true
-
-query-sql
-SELECT node, locality, ok, error, can_delete FROM [SHOW BACKUP CONNECTION 'nodelocal://1/conn-test' WITH TRANSFER='1', TIME = '1ms'] ORDER BY node;
-----
-1 region=eu-central-1,availability-zone=eu-central-1 true  true
-2 region=eu-north-1,availability-zone=eu-north-1 true  true
-3 region=us-east-1,availability-zone=us-east1 true  true