Merge #148008 #148217

148008: roachprod: improve instances filtering when listing on IBM r=DarrylWong a=golgeek

Cloud instance listing on IBM was previously filtering out VMs that lacked the `roachprod` tag. However, the IBM API occasionally returns a successful response for tagging requests without actually acting on it. This inconsistency resulted in some VMs being skipped in the garbage colelction process because lacking the required tags.

This PR removes the dependency on the `roachprod` tag, and all resources within the `roachprod` resource group are now considered game for garbage collection.

Epic: none
Release note: None

148217: sql: Index routine create virtual tables by routine IDs. r=OriSavir a=OriSavir

Previously, the internal routine virtual tables,
`crdb_internal.create_function_statements` and
`crdb_internal.create_procedure_statements` were
not indexed. As such, in the functions to query for a routine create statement by the routine ID,
which are necessary for `SHOW CREATE ALL ROUTINES`, the query was on a virtual table that was not indexed and, thus, inefficient. This commit addresses that by adding a function that can be used to build rows of the virtual tables directly from the routine IDs and, thus, can act as a populate function for virtual indexes. This function was added onto the virtual tables and now the query is more efficient.

Epic: CRDB-49582
Release note: None

Co-authored-by: Ludovic Leroux <[email protected]>
Co-authored-by: Oriel Savir <[email protected]>

Author: 3 people

pkg/bench/rttanalysis/orm_queries_bench_test.go

@@ -1027,6 +1027,14 @@ func buildNTables(n int) string {
 	return b.String()
 }
 
+func buildNFunctions(n int) string {
+	b := strings.Builder{}
+	for i := 0; i < n; i++ {
+		b.WriteString(fmt.Sprintf("CREATE FUNCTION fn%d() RETURNS int AS 'SELECT 1' LANGUAGE SQL;\n", i))
+	}
+	return b.String()
+}
+
 func buildNTypes(n int) string {
 	b := strings.Builder{}
 	for i := 0; i < n; i++ {

pkg/bench/rttanalysis/testdata/benchmark_expectations

@@ -131,5 +131,6 @@ exp,benchmark
 2,UseManyRoles/use_50_roles
 1,VirtualTableQueries/select_crdb_internal.invalid_objects_with_1_fk
 1,VirtualTableQueries/select_crdb_internal.tables_with_1_fk
+0,VirtualTableQueries/show_create_all_routines
 5,VirtualTableQueries/virtual_table_cache_with_point_lookups
 10,VirtualTableQueries/virtual_table_cache_with_schema_change

pkg/bench/rttanalysis/virtual_table_bench_test.go

@@ -59,5 +59,12 @@ SELECT * FROM crdb_internal.tables;
 SELECT * FROM t1;
 SELECT * FROM t2;`,
 		},
+		// This test checks the performance of the crdb_internal virtual tables used by
+		// SHOW CREATE ALL ROUTINES.
+		{
+			Name:  "show_create_all_routines",
+			Setup: buildNFunctions(100),
+			Stmt:  `SHOW CREATE ALL ROUTINES`,
+		},
 	})
 }

pkg/roachprod/vm/ibm/helpers.go

@@ -833,7 +833,7 @@ func (p *Provider) listRegion(l *logger.Logger, r string, opts vm.ListOptions) (
 			}
 		}
 
-		valid, reason := i.isRoachprodAndValidStatus()
+		valid, reason := i.isValidStatus()
 		if !valid {
 			l.Printf("WARN: discarding instance %s in region %s (%s)", *i.instance.CRN, r, reason)
 			continue

pkg/roachprod/vm/ibm/ibm_extended_types.go

@@ -485,25 +485,11 @@ func (i *instance) getPrimaryNetworkInterface() (*vpcv1NetworkInterface, error)
 	return i.networkInterface, nil
 }
 
-// isRoachprodAndValidStatus checks if a roahcprod instance in valid status.
-func (i *instance) isRoachprodAndValidStatus() (bool, string) {
+// isValidStatus checks if an instance has a valid status.
+func (i *instance) isValidStatus() (bool, string) {
 	if i == nil || i.instance == nil || i.instance.Status == nil {
 		return false, InstanceNotInitialized
 	}
-
-	tags, err := i.getTagsAsMap()
-	if err != nil {
-		return false, InstanceInvalidTags
-	}
-
-	if tags[vm.TagRoachprod] != "true" {
-		return false, InstanceNotRochprod
-	}
-
-	// if *i.instance.Status != "pending" && *i.instance.Status != "running" {
-	// 	return false, InstanceInvalidStatus
-	// }
-
 	return true, ""
 }
 
@@ -585,6 +571,11 @@ func (i *instance) toVM() vm.VM {
 		vpcID = i.provider.config.regions[region].vpcID
 	}
 
+	// Check if the instance is in a valid state.
+	if core.StringNilMapper(i.instance.Status) == "failed" {
+		vmErrors = append(vmErrors, errors.New("instance is in failed state"))
+	}
+
 	// Gather tags
 	tags, err := i.getTagsAsMap()
 	if err != nil {

pkg/sql/crdb_internal.go

@@ -3912,6 +3912,79 @@ func createRoutinePopulate(
 	}
 }
 
+func createRoutinePopulateByFnIndex(
+	ctx context.Context,
+	unwrappedConstraint tree.Datum,
+	p *planner,
+	db catalog.DatabaseDescriptor,
+	addRow func(...tree.Datum) error,
+) (matched bool, err error) {
+	// In here, we start from the DatabaseDescriptor and a routine ID and populate the remainder of a row
+	// in the virtual table. This function, thus, is used to populate when indexing into the routine virtual tables,
+	// `crdb_internal.create_function_statements` and `crdb_internal.create_procedure_statements`, helping
+	// optimize the queries for the create statements output in `SHOW CREATE ALL ROUTINES`.
+	fnID := descpb.ID(tree.MustBeDInt(unwrappedConstraint))
+	fnDesc, err := p.Descriptors().ByIDWithoutLeased(p.txn).WithoutNonPublic().Get().Function(ctx, fnID)
+	if err != nil || fnDesc == nil {
+		if errors.Is(err, catalog.ErrDescriptorNotFound) || fnDesc == nil {
+			return false, nil
+		}
+		return false, err
+	}
+	scID := fnDesc.GetParentSchemaID()
+	sc, err := p.Descriptors().ByIDWithoutLeased(p.txn).WithoutNonPublic().Get().Schema(ctx, scID)
+	if err != nil || sc == nil {
+		return false, err
+	}
+	scName := sc.GetName()
+	dbName := db.GetName()
+	dbID := db.GetID()
+
+	treeNode, err := fnDesc.ToCreateExpr()
+	treeNode.Name.ObjectNamePrefix = tree.ObjectNamePrefix{
+		ExplicitSchema: true,
+		SchemaName:     tree.Name(scName),
+	}
+	if err != nil {
+		return false, err
+	}
+	for i := range treeNode.Options {
+		if body, ok := treeNode.Options[i].(tree.RoutineBodyStr); ok {
+			bodyStr := string(body)
+			bodyStr, err = formatFunctionQueryTypesForDisplay(ctx, p.EvalContext(), &p.semaCtx, p.SessionData(), bodyStr, fnDesc.GetLanguage())
+			if err != nil {
+				return false, err
+			}
+			bodyStr, err = formatQuerySequencesForDisplay(ctx, &p.semaCtx, bodyStr, true /* multiStmt */, fnDesc.GetLanguage())
+			if err != nil {
+				return false, err
+			}
+			bodyStr = strings.TrimSpace(bodyStr)
+			stmtStrs := strings.Split(bodyStr, "\n")
+			for i := range stmtStrs {
+				if stmtStrs[i] != "" {
+					stmtStrs[i] = "\t" + stmtStrs[i]
+				}
+			}
+			p := &treeNode.Options[i]
+			*p = tree.RoutineBodyStr("\n" + strings.Join(stmtStrs, "\n") + "\n")
+		}
+	}
+	createStatement := tree.AsString(treeNode)
+	if err := addRow(
+		tree.NewDInt(tree.DInt(dbID)),           // database_id
+		tree.NewDString(dbName),                 // database_name
+		tree.NewDInt(tree.DInt(scID)),           // schema_id
+		tree.NewDString(scName),                 // schema_name
+		tree.NewDInt(tree.DInt(fnDesc.GetID())), // function_id
+		tree.NewDString(fnDesc.GetName()),       // function_name
+		tree.NewDString(createStatement),        // create_statement
+	); err != nil {
+		return false, err
+	}
+	return true, nil
+}
+
 var crdbInternalCreateFunctionStmtsTable = virtualSchemaTable{
 	comment: "CREATE statements for all user-defined functions",
 	schema: `
@@ -3922,10 +3995,15 @@ CREATE TABLE crdb_internal.create_function_statements (
   schema_name STRING,
   function_id INT,
   function_name STRING,
-  create_statement STRING
-)
-`,
+  create_statement STRING,
+  INDEX (function_id)
+)`,
 	populate: createRoutinePopulate(false /* procedure */),
+	indexes: []virtualIndex{
+		{
+			populate: createRoutinePopulateByFnIndex,
+		},
+	},
 }
 
 var crdbInternalCreateProcedureStmtsTable = virtualSchemaTable{
@@ -3938,10 +4016,15 @@ CREATE TABLE crdb_internal.create_procedure_statements (
   schema_name STRING,
   procedure_id INT,
   procedure_name STRING,
-  create_statement STRING
-)
-`,
+  create_statement STRING,
+  INDEX (procedure_id)
+)`,
 	populate: createRoutinePopulate(true /* procedure */),
+	indexes: []virtualIndex{
+		{
+			populate: createRoutinePopulateByFnIndex,
+		},
+	},
 }
 
 func createTriggerPopulate(

pkg/sql/opt/exec/execbuilder/testdata/explain_analyze

@@ -176,7 +176,7 @@ quality of service: regular
         │       │               └── • virtual table
         │       │                     sql nodes: <hidden>
         │       │                     regions: <hidden>
-        │       │                     actual row count: 364
+        │       │                     actual row count: 366
         │       │                     execution time: 0µs
         │       │                     table: pg_class@primary
         │       │