tpcc/roachtest: adjust tpcc workload to optionally include deprecated…

… FKs

Fixes #53136.
Fixes #53122.

Updates the TPCC workload to create no longer needed FK indexes based on
a flag, and use that flag when running tpcc on versions below 20.2.

Release note: None

pkg/cmd/roachtest/import.go

@@ -17,6 +17,7 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/util/retry"
+	"github.com/cockroachdb/cockroach/pkg/util/version"
 	"github.com/cockroachdb/errors"
 )
 
@@ -35,12 +36,15 @@ func registerImportTPCC(r *testRegistry) {
 		hc := NewHealthChecker(c, c.All())
 		m.Go(hc.Runner)
 
+		workloadStr := `./workload fixtures import tpcc --warehouses=%d --csv-server='http://localhost:8081'`
+		if !t.buildVersion.AtLeast(version.MustParse("v20.2.0")) {
+			workloadStr += " --deprecated-fk-indexes"
+		}
+
 		m.Go(func(ctx context.Context) error {
 			defer dul.Done()
 			defer hc.Done()
-			cmd := fmt.Sprintf(
-				`./workload fixtures import tpcc --warehouses=%d --csv-server='http://localhost:8081'`,
-				warehouses)
+			cmd := fmt.Sprintf(workloadStr, warehouses)
 			c.Run(ctx, c.Node(1), cmd)
 			return nil
 		})

pkg/cmd/roachtest/version.go

@@ -18,20 +18,21 @@ import (
 	"time"
 
 	"github.com/cockroachdb/cockroach/pkg/util/binfetcher"
+	"github.com/cockroachdb/cockroach/pkg/util/version"
 	"github.com/cockroachdb/errors"
 	_ "github.com/lib/pq"
 )
 
 // TODO(tbg): remove this test. Use the harness in versionupgrade.go
 // to make a much better one, much more easily.
 func registerVersion(r *testRegistry) {
-	runVersion := func(ctx context.Context, t *test, c *cluster, version string) {
+	runVersion := func(ctx context.Context, t *test, c *cluster, binaryVersion string) {
 		nodes := c.spec.NodeCount - 1
 		goos := ifLocal(runtime.GOOS, "linux")
 
 		b, err := binfetcher.Download(ctx, binfetcher.Options{
 			Binary:  "cockroach",
-			Version: "v" + version,
+			Version: "v" + binaryVersion,
 			GOOS:    goos,
 			GOARCH:  "amd64",
 		})
@@ -56,8 +57,13 @@ func registerVersion(r *testRegistry) {
 
 		loadDuration := " --duration=" + (time.Duration(3*nodes+2)*stageDuration + buffer).String()
 
+		var deprecatedWorkloadsStr string
+		if !t.buildVersion.AtLeast(version.MustParse("v20.2.0")) {
+			deprecatedWorkloadsStr += " --deprecated-fk-indexes"
+		}
+
 		workloads := []string{
-			"./workload run tpcc --tolerate-errors --wait=false --drop --init --warehouses=1 " + loadDuration + " {pgurl:1-%d}",
+			"./workload run tpcc --tolerate-errors --wait=false --drop --init --warehouses=1 " + deprecatedWorkloadsStr + loadDuration + " {pgurl:1-%d}",
 			"./workload run kv --tolerate-errors --init" + loadDuration + " {pgurl:1-%d}",
 		}
 
@@ -101,7 +107,7 @@ func registerVersion(r *testRegistry) {
 					// checks had been broken for a long time. See:
 					//
 					// https://github.com/cockroachdb/cockroach/issues/37737#issuecomment-496026918
-					if !strings.HasPrefix(version, "2.") {
+					if !strings.HasPrefix(binaryVersion, "2.") {
 						if err := c.CheckReplicaDivergenceOnDB(ctx, db); err != nil {
 							return errors.Wrapf(err, "node %d", i)
 						}

pkg/workload/cli/cli.go

@@ -14,6 +14,7 @@ import (
 	"os"
 
 	"github.com/cockroachdb/cockroach/pkg/workload"
+	"github.com/cockroachdb/errors"
 	"github.com/spf13/cobra"
 )
 
@@ -73,7 +74,11 @@ func HandleErrs(
 	return func(cmd *cobra.Command, args []string) {
 		err := f(cmd, args)
 		if err != nil {
+			hint := errors.FlattenHints(err)
 			cmd.Println("Error:", err.Error())
+			if hint != "" {
+				cmd.Println("Hint:", hint)
+			}
 			os.Exit(1)
 		}
 	}

pkg/workload/tpcc/ddls.go

@@ -90,8 +90,10 @@ const (
 		h_date   timestamp,
 		h_amount decimal(6,2),
 		h_data   varchar(24),
-		primary key (h_w_id, rowid)
-  )`
+		primary key (h_w_id, rowid)`
+	deprecatedTpccHistorySchemaFkSuffix = `
+		index history_customer_fk_idx (h_c_w_id, h_c_d_id, h_c_id),
+		index history_district_fk_idx (h_w_id, h_d_id)`
 
 	// ORDER table.
 	tpccOrderSchemaBase = `(
@@ -169,8 +171,9 @@ const (
 		ol_quantity     integer,
 		ol_amount       decimal(6,2),
 		ol_dist_info    char(24),
-		primary key (ol_w_id, ol_d_id, ol_o_id DESC, ol_number)
-  )`
+		primary key (ol_w_id, ol_d_id, ol_o_id DESC, ol_number)`
+	deprecatedTpccOrderLineSchemaFkSuffix = `
+		index order_line_stock_fk_idx (ol_supply_w_id, ol_i_id)`
 	tpccOrderLineSchemaInterleaveSuffix = `
 		interleave in parent "order" (ol_w_id, ol_d_id, ol_o_id)`
 )

pkg/workload/tpcc/tpcc.go

@@ -53,7 +53,10 @@ type tpcc struct {
 	waitFraction float64
 	workers      int
 	fks          bool
-	dbOverride   string
+	// deprecatedFKIndexes adds in foreign key indexes that are no longer needed
+	// due to origin index restrictions being lifted.
+	deprecatedFkIndexes bool
+	dbOverride          string
 
 	txInfos []txInfo
 	// deck contains indexes into the txInfos slice.
@@ -164,6 +167,7 @@ var tpccMeta = workload.Meta{
 		g.flags.Uint64Var(&g.seed, `seed`, 1, `Random number generator seed`)
 		g.flags.IntVar(&g.warehouses, `warehouses`, 1, `Number of warehouses for loading`)
 		g.flags.BoolVar(&g.fks, `fks`, true, `Add the foreign keys`)
+		g.flags.BoolVar(&g.deprecatedFkIndexes, `deprecated-fk-indexes`, false, `Add deprecated foreign keys (needed when running against v20.1 or below clusters)`)
 		g.flags.BoolVar(&g.interleaved, `interleaved`, false, `Use interleaved tables`)
 
 		g.flags.StringVar(&g.mix, `mix`,
@@ -326,10 +330,18 @@ func (w *tpcc) Hooks() workload.Hooks {
 
 				for _, fkStmt := range fkStmts {
 					if _, err := db.Exec(fkStmt); err != nil {
-						// If the statement failed because the fk already exists,
-						// ignore it. Return the error for any other reason.
 						const duplFKErr = "columns cannot be used by multiple foreign key constraints"
-						if !strings.Contains(err.Error(), duplFKErr) {
+						const idxErr = "foreign key requires an existing index on columns"
+						switch {
+						case strings.Contains(err.Error(), idxErr):
+							fmt.Println(errors.WithHint(err, "try using the --deprecated-fk-indexes flag"))
+							// If the statement failed because of a missing FK index, suggest
+							// to use the deprecated-fks flag.
+							return errors.WithHint(err, "try using the --deprecated-fk-indexes flag")
+						case strings.Contains(err.Error(), duplFKErr):
+							// If the statement failed because the fk already exists,
+							// ignore it. Return the error for any other reason.
+						default:
 							return err
 						}
 					}
@@ -466,8 +478,12 @@ func (w *tpcc) Tables() []workload.Table {
 		Stats: w.tpccCustomerStats(),
 	}
 	history := workload.Table{
-		Name:   `history`,
-		Schema: tpccHistorySchemaBase,
+		Name: `history`,
+		Schema: maybeAddFkSuffix(
+			w.deprecatedFkIndexes,
+			tpccHistorySchemaBase,
+			deprecatedTpccHistorySchemaFkSuffix,
+		),
 		InitialRows: workload.BatchedTuples{
 			NumBatches: numHistoryPerWarehouse * w.warehouses,
 			FillBatch:  w.tpccHistoryInitialRowBatch,
@@ -536,7 +552,11 @@ func (w *tpcc) Tables() []workload.Table {
 		Name: `order_line`,
 		Schema: maybeAddInterleaveSuffix(
 			w.interleaved,
-			tpccOrderLineSchemaBase,
+			maybeAddFkSuffix(
+				w.deprecatedFkIndexes,
+				tpccOrderLineSchemaBase,
+				deprecatedTpccOrderLineSchemaFkSuffix,
+			),
 			tpccOrderLineSchemaInterleaveSuffix,
 		),
 		InitialRows: workload.BatchedTuples{