Merge #52015 #52040

52015: sql: use a structured error to detect roachpb.BatchTimestampBeforeGCE… r=ajwerner a=ajwerner

…rror

Fixes #50198.

See the issue for more details. This issue is so obscure it does not deserve
a release note.

Release note: None

52040: roachtest: fix release-20.1 roachtests failing due to double-init r=irfansharif a=irfansharif

Fixes #51965 (and all referencing issues).

Roachprod clusters running v20.1+ crdb nodes persist this
`cluster-bootstrapped` file on disk after explicitly bootstrapping the
cluster. Roachprod then uses the existence of this file to avoid doubly
bootstrapping the cluster.

Given #51897 remains unresolved, master-built roachprod is used to run
roachtests against the 20.1 branch. Some of those roachtests test
mixed-version clusters that start off at 19.2. Consequently, we manually
add this file where roachprod expects to find it for already-initialized
clusters. (This is a pretty gross hack, that we should address by
addressing #51897.)

Release note: None

Co-authored-by: Andrew Werner <ajwerner@cockroachlabs.com>
Co-authored-by: irfan sharif <irfanmahmoudsharif@gmail.com>

pkg/cmd/roachprod/install/cockroach.go

@@ -168,6 +168,26 @@ func (r Cockroach) Start(c *SyncedCluster, extraArgs []string) {
 			}
 		}
 
+		if !vers.AtLeast(version.MustParse("v20.1.0")) {
+			// Given #51897 remains unresolved, master-built roachprod is used
+			// to run roachtests against the 20.1 branch. Some of those
+			// roachtests test mixed-version clusters that start off at 19.2.
+			// Consequently, we manually add this `cluster-bootstrapped` file
+			// where roachprod expects to find it for already-initialized
+			// clusters. This is a pretty gross hack, that we should address by
+			// addressing #51897.
+			//
+			// TODO(irfansharif): Remove this once #51897 is resolved.
+			markBootstrap := fmt.Sprintf("touch %s/%s", h.c.Impl.NodeDir(h.c, nodes[nodeIdx]), "cluster-bootstrapped")
+			cmdOut, err := h.run(nodeIdx, markBootstrap)
+			if err != nil {
+				log.Fatalf("unable to run cmd: %v", err)
+			}
+			if cmdOut != "" {
+				fmt.Println(cmdOut)
+			}
+		}
+
 		fmt.Printf("%s: setting cluster settings", h.c.Name)
 		clusterSettingsOut, err := h.setClusterSettings(nodeIdx)
 		if err != nil {
@@ -619,3 +639,19 @@ func (h *crdbStartHelper) getEnvVars() string {
 	}
 	return buf.String()
 }
+
+func (h *crdbStartHelper) run(nodeIdx int, cmd string) (string, error) {
+	nodes := h.c.ServerNodes()
+
+	sess, err := h.c.newSession(nodes[nodeIdx])
+	if err != nil {
+		return "", err
+	}
+	defer sess.Close()
+
+	out, err := sess.CombinedOutput(cmd)
+	if err != nil {
+		return "", errors.Wrapf(err, "~ %s\n%s", cmd, out)
+	}
+	return strings.TrimSpace(string(out)), nil
+}

pkg/cmd/roachtest/main.go

@@ -161,6 +161,10 @@ the test tags.
 		},
 	}
 
+	// TODO(irfansharif): We could remove this by directly running `cockroach
+	// version` against the binary being tested, instead of what we do today
+	// which is defaulting to checking the last git release tag present in the
+	// local checkout.
 	runCmd.Flags().StringVar(
 		&buildTag, "build-tag", "", "build tag (auto-detect if empty)")
 	runCmd.Flags().StringVar(

pkg/sql/schema_changer.go

@@ -134,10 +134,7 @@ func isPermanentSchemaChangeError(err error) bool {
 	// Ignore error thrown because of a read at a very old timestamp.
 	// The Backfill will grab a new timestamp to read at for the rest
 	// of the backfill.
-	// TODO(knz): this should really use errors.Is(). However until/unless
-	// we are not receiving errors from 19.1 any more, a string
-	// comparison must remain.
-	if strings.Contains(err.Error(), "must be after replica GC threshold") {
+	if errors.HasType(err, (*roachpb.BatchTimestampBeforeGCError)(nil)) {
 		return false
 	}