Merge #72281 #75471 #75472 #75553

72281: sql: add new BACKFILLING mutation state r=postamar a=stevendanna

This adds a BACKFILLING mutation state. When an index is in this
state, it is ignored for the purposes of INSERT, UPDATE, and
DELETE. Currently, this state is unused in the actual code.

This new state will eventually be used in the new index backfiller. In
the new backfiller, the bulk-backfill using AddSSTable will occur when
the index is in this state, with concurrent updates being captured by
a temporary index. See
docs/RFCS/20211004_incremental_index_backfiller.md for more details.

Indexes in this state are returned by `(TableDescriptor).AllIndexes`, 
`(TableDescriptor).NonDropIndexes`, and `(TableDescriptor).Partial`.

Release note: None

75471: scripts/gceworker.sh: remove extra s in sleep command r=rail a=mufeez-amjad

Release note: None

75472: ci: add script for compose nightly, dev support r=rail a=rickystewart

Closes #67151.
Closes #70893.

Release note: None

75553: distsql: bump size of test r=rail a=rickystewart

This has timed out in CI.

Release note: None

Co-authored-by: Steven Danna <[email protected]>
Co-authored-by: Mufeez Amjad <[email protected]>
Co-authored-by: Ricky Stewart <[email protected]>

build/teamcity/cockroach/nightlies/compose.sh

@@ -0,0 +1,18 @@
+#!/usr/bin/env bash
+
+set -xeuo pipefail
+
+dir="$(dirname $(dirname $(dirname $(dirname "${0}"))))"
+source "$dir/teamcity-support.sh"
+
+tc_start_block "Run compose tests"
+
+bazel build //pkg/cmd/bazci --config=ci
+BAZCI=$(bazel info bazel-bin --config=ci)/pkg/cmd/bazci/bazci_/bazci
+
+$BAZCI run --config=crosslinux --config=test --config=with_ui --artifacts_dir=$PWD/artifacts \
+       //pkg/compose:compose_test -- \
+       --test_env=GO_TEST_WRAP_TESTV=1 \
+       --test_timeout=1800
+
+tc_end_block "Run compose tests"

pkg/build/bazel/non_bazel.go

@@ -21,6 +21,11 @@ func BuiltWithBazel() bool {
 	return false
 }
 
+// FindBinary is not implemented.
+func FindBinary(pkg, name string) (string, bool) {
+	panic("not build with Bazel")
+}
+
 // Runfile is not implemented.
 func Runfile(string) (string, error) {
 	panic("not built with Bazel")

pkg/cmd/dev/BUILD.bazel

@@ -7,6 +7,7 @@ go_library(
         "bench.go",
         "build.go",
         "builder.go",
+        "compose.go",
         "dev.go",
         "doctor.go",
         "generate.go",

pkg/cmd/dev/acceptance.go

@@ -34,7 +34,6 @@ func makeAcceptanceCmd(runE func(cmd *cobra.Command, args []string) error) *cobr
 func (d *dev) acceptance(cmd *cobra.Command, _ []string) error {
 	ctx := cmd.Context()
 	var (
-		// cpus, remote-cache
 		filter  = mustGetFlagString(cmd, filterFlag)
 		short   = mustGetFlagBool(cmd, shortFlag)
 		timeout = mustGetFlagDuration(cmd, timeoutFlag)

pkg/cmd/dev/compose.go

@@ -0,0 +1,59 @@
+// Copyright 2022 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+package main
+
+import (
+	"fmt"
+
+	"github.com/spf13/cobra"
+)
+
+func makeComposeCmd(runE func(cmd *cobra.Command, args []string) error) *cobra.Command {
+	composeCmd := &cobra.Command{
+		Use:     "compose",
+		Short:   "Run compose tests",
+		Long:    "Run compose tests.",
+		Example: "dev compose",
+		Args:    cobra.ExactArgs(0),
+		RunE:    runE,
+	}
+	addCommonBuildFlags(composeCmd)
+	addCommonTestFlags(composeCmd)
+	return composeCmd
+}
+
+func (d *dev) compose(cmd *cobra.Command, _ []string) error {
+	ctx := cmd.Context()
+	var (
+		filter  = mustGetFlagString(cmd, filterFlag)
+		short   = mustGetFlagBool(cmd, shortFlag)
+		timeout = mustGetFlagDuration(cmd, timeoutFlag)
+	)
+
+	var args []string
+	args = append(args, "run", "//pkg/compose:compose_test", "--config=test")
+	args = append(args, mustGetRemoteCacheArgs(remoteCacheAddr)...)
+	if numCPUs != 0 {
+		args = append(args, fmt.Sprintf("--local_cpu_resources=%d", numCPUs))
+	}
+	if filter != "" {
+		args = append(args, fmt.Sprintf("--test_filter=%s", filter))
+	}
+	if short {
+		args = append(args, "--test_arg", "-test.short")
+	}
+	if timeout > 0 {
+		args = append(args, fmt.Sprintf("--test_timeout=%d", int(timeout.Seconds())))
+	}
+
+	logCommand("bazel", args...)
+	return d.exec.CommandContextInheritingStdStreams(ctx, "bazel", args...)
+}

pkg/cmd/dev/dev.go

@@ -112,6 +112,7 @@ Typical usage:
 		makeBenchCmd(ret.bench),
 		makeBuildCmd(ret.build),
 		makeBuilderCmd(ret.builder),
+		makeComposeCmd(ret.compose),
 		makeDoctorCmd(ret.doctor),
 		makeGenerateCmd(ret.generate),
 		makeGoCmd(ret.gocmd),

pkg/compose/BUILD.bazel

@@ -10,7 +10,11 @@ go_library(
 go_test(
     name = "compose_test",
     srcs = ["compose_test.go"],
-    data = ["//pkg/compose:compare/docker-compose.yml"],
+    data = [
+        "//pkg/cmd/cockroach",
+        "//pkg/compose:compare/docker-compose.yml",
+        "//pkg/compose/compare/compare:compare_test",
+    ],
     embed = [":compose"],
     gotags = ["compose"],
     tags = ["integration"],

pkg/compose/compare/compare/BUILD.bazel

@@ -13,6 +13,7 @@ go_test(
     embed = [":compare"],
     gotags = ["compose"],
     tags = ["integration"],
+    visibility = ["//pkg/compose:__subpackages__"],
     deps = [
         "//pkg/cmd/cmpconn",
         "//pkg/internal/sqlsmith",

pkg/compose/compare/docker-compose.yml

@@ -9,19 +9,19 @@ services:
     image: ubuntu:xenial-20170214
     command: /cockroach/cockroach start-single-node --insecure --listen-addr cockroach1
     volumes:
-      - ../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach
+      - "${COCKROACH_PATH}:/cockroach/cockroach"
   cockroach2:
     image: ubuntu:xenial-20170214
     command: /cockroach/cockroach start-single-node --insecure --listen-addr cockroach2
     volumes:
-      - ../../../cockroach-linux-2.6.32-gnu-amd64:/cockroach/cockroach
+      - "${COCKROACH_PATH}:/cockroach/cockroach"
   test:
     image: ubuntu:xenial-20170214
-    # compare.test is a binary built by the pkg/compose/prepare.sh
-    command: /compare/compare.test -each ${EACH} -test.run ${TESTS} -artifacts /compare
+    # compare.test is a binary built by the pkg/compose/prepare.sh in non-bazel builds
+    command: /compare/compare.test -each ${EACH} -test.run ${TESTS} -artifacts ${ARTIFACTS}
     depends_on:
       - postgres
       - cockroach1
       - cockroach2
     volumes:
-      - ./compare:/compare
+      - "${COMPARE_DIR_PATH}:/compare"

pkg/compose/compose_test.go

@@ -20,6 +20,9 @@ package compose
 import (
 	"flag"
 	"fmt"
+	"io"
+	"io/ioutil"
+	"os"
 	"os/exec"
 	"path/filepath"
 	"testing"
@@ -29,20 +32,74 @@ import (
 )
 
 var (
-	flagEach  = flag.Duration("each", 10*time.Minute, "individual test timeout")
-	flagTests = flag.String("tests", ".", "tests within docker compose to run")
+	flagEach      = flag.Duration("each", 10*time.Minute, "individual test timeout")
+	flagTests     = flag.String("tests", ".", "tests within docker compose to run")
+	flagArtifacts = flag.String("artifacts", "", "artifact directory")
 )
 
+func copyBin(src, dst string) error {
+	in, err := os.Open(src)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = in.Close() }()
+	out, err := os.OpenFile(dst, os.O_RDWR|os.O_CREATE, 0755)
+	if err != nil {
+		return err
+	}
+	defer func() { _ = out.Close() }()
+	_, err = io.Copy(out, in)
+	return err
+}
+
 func TestComposeCompare(t *testing.T) {
-	var dockerComposeYml string
+	var cockroachBin, compareDir, dockerComposeYml string
 	if bazel.BuiltWithBazel() {
 		var err error
 		dockerComposeYml, err = bazel.Runfile("pkg/compose/compare/docker-compose.yml")
 		if err != nil {
 			t.Fatal(err)
 		}
+		origCockroachBin, found := bazel.FindBinary("pkg/cmd/cockroach", "cockroach")
+		if !found {
+			t.Fatal("could not find cockroach binary")
+		}
+		origCompareBin, found := bazel.FindBinary("pkg/compose/compare/compare", "compare_test")
+		if !found {
+			t.Fatal("could not find compare_test binary")
+		}
+		// These binaries are going to be mounted as volumes when we
+		// start up docker-compose, but the files themselves will be
+		// Bazel-built symlinks. We want to copy these files to a
+		// different temporary location.
+		composeBinsDir, err := ioutil.TempDir("", "compose-bins")
+		if err != nil {
+			t.Fatal(err)
+		}
+		defer func() { _ = os.RemoveAll(composeBinsDir) }()
+		compareDir = composeBinsDir
+		cockroachBin = filepath.Join(composeBinsDir, "cockroach")
+		err = copyBin(origCockroachBin, cockroachBin)
+		if err != nil {
+			t.Fatal(err)
+		}
+		err = copyBin(origCompareBin, filepath.Join(composeBinsDir, "compare.test"))
+		if err != nil {
+			t.Fatal(err)
+		}
+		if *flagArtifacts == "" {
+			*flagArtifacts, err = ioutil.TempDir("", "compose")
+			if err != nil {
+				t.Fatal(err)
+			}
+		}
 	} else {
+		cockroachBin = "../../../cockroach-linux-2.6.32-gnu-amd64"
+		compareDir = "./compare"
 		dockerComposeYml = filepath.Join("compare", "docker-compose.yml")
+		if *flagArtifacts == "" {
+			*flagArtifacts = compareDir
+		}
 	}
 	cmd := exec.Command(
 		"docker-compose",
@@ -55,6 +112,9 @@ func TestComposeCompare(t *testing.T) {
 	cmd.Env = []string{
 		fmt.Sprintf("EACH=%s", *flagEach),
 		fmt.Sprintf("TESTS=%s", *flagTests),
+		fmt.Sprintf("COCKROACH_PATH=%s", cockroachBin),
+		fmt.Sprintf("COMPARE_DIR_PATH=%s", compareDir),
+		fmt.Sprintf("ARTIFACTS=%s", *flagArtifacts),
 	}
 	out, err := cmd.CombinedOutput()
 	if err != nil {

pkg/sql/catalog/descpb/structured.proto

@@ -673,6 +673,19 @@ message DescriptorMutation {
     // (column default or index data) can only be backfilled once
     // all nodes have transitioned into the DELETE_AND_WRITE_ONLY state.
     DELETE_AND_WRITE_ONLY = 2;
+
+    // Operations other than non-transactional backfilling must not
+    // use this descriptor.
+    //
+    // Column: Columns do not use this state. A column descriptor in
+    // this state is a programming error.
+    //
+    // Index: A descriptor in this state is invisible to an INSERT,
+    // UPDATE, and DELETE.
+    //
+    // TODO(ssd): This is currently unused and is being added to
+    // facilitate the new temporary-index-based backfilling process.
+    BACKFILLING = 3;
   }
   optional State state = 3 [(gogoproto.nullable) = false];
 

pkg/sql/catalog/descriptor.go

@@ -385,10 +385,18 @@ type TableDescriptor interface {
 
 	// DeletableNonPrimaryIndexes returns a slice of all non-primary indexes
 	// which allow being deleted from: public + delete-and-write-only +
-	// delete-only, in  their canonical order. This is equivalent to taking
-	// the slice produced by AllIndexes and removing the primary index.
+	// delete-only, in their canonical order. This is equivalent to taking
+	// the slice produced by AllIndexes and removing the primary index and
+	// backfilling indexes.
 	DeletableNonPrimaryIndexes() []Index
 
+	// NonPrimaryIndexes returns a slice of all the indexes that
+	// are not yet public: backfilling, delete, and
+	// delete-and-write-only, in their canonical order. This is
+	// equivalent to taking the slice produced by AllIndexes and
+	// removing the primary index.
+	NonPrimaryIndexes() []Index
+
 	// DeleteOnlyNonPrimaryIndexes returns a slice of all non-primary indexes
 	// which allow only being deleted from, in their canonical order. This is
 	// equivalent to taking the slice produced by DeletableNonPrimaryIndexes and

pkg/sql/catalog/table_elements.go

@@ -41,6 +41,10 @@ type TableElementMaybeMutation interface {
 	// delete-only state.
 	DeleteOnly() bool
 
+	// Backfilling returns true iff the table element is in a
+	// mutation in the backfilling state.
+	Backfilling() bool
+
 	// Adding returns true iff the table element is in an add mutation.
 	Adding() bool
 
@@ -529,6 +533,12 @@ func ForEachPartialIndex(desc TableDescriptor, f func(idx Index) error) error {
 	return forEachIndex(desc.PartialIndexes(), f)
 }
 
+// ForEachNonPrimaryIndex is like ForEachIndex over
+// NonPrimaryIndexes().
+func ForEachNonPrimaryIndex(desc TableDescriptor, f func(idx Index) error) error {
+	return forEachIndex(desc.NonPrimaryIndexes(), f)
+}
+
 // ForEachPublicNonPrimaryIndex is like ForEachIndex over
 // PublicNonPrimaryIndexes().
 func ForEachPublicNonPrimaryIndex(desc TableDescriptor, f func(idx Index) error) error {
@@ -613,6 +623,12 @@ func FindDeletableNonPrimaryIndex(desc TableDescriptor, test func(idx Index) boo
 	return findIndex(desc.DeletableNonPrimaryIndexes(), test)
 }
 
+// FindNonPrimaryIndex returns the first index in
+// NonPrimaryIndex() for which test returns true.
+func FindNonPrimaryIndex(desc TableDescriptor, test func(idx Index) bool) Index {
+	return findIndex(desc.NonPrimaryIndexes(), test)
+}
+
 // FindDeleteOnlyNonPrimaryIndex returns the first index in
 // DeleteOnlyNonPrimaryIndex() for which test returns true.
 func FindDeleteOnlyNonPrimaryIndex(desc TableDescriptor, test func(idx Index) bool) Index {

pkg/sql/catalog/tabledesc/index.go

@@ -458,6 +458,7 @@ type indexCache struct {
 	all                  []catalog.Index
 	active               []catalog.Index
 	nonDrop              []catalog.Index
+	nonPrimary           []catalog.Index
 	publicNonPrimary     []catalog.Index
 	writableNonPrimary   []catalog.Index
 	deletableNonPrimary  []catalog.Index
@@ -490,7 +491,13 @@ func newIndexCache(desc *descpb.TableDescriptor, mutations *mutationCache) *inde
 	c.primary = c.all[0]
 	c.active = c.all[:numPublic]
 	c.publicNonPrimary = c.active[1:]
-	c.deletableNonPrimary = c.all[1:]
+	for _, idx := range c.all[1:] {
+		if !idx.Backfilling() {
+			lazyAllocAppendIndex(&c.deletableNonPrimary, idx, len(c.all[1:]))
+		}
+		lazyAllocAppendIndex(&c.nonPrimary, idx, len(c.all[1:]))
+	}
+
 	if numMutations == 0 {
 		c.writableNonPrimary = c.publicNonPrimary
 	} else {
@@ -506,7 +513,11 @@ func newIndexCache(desc *descpb.TableDescriptor, mutations *mutationCache) *inde
 		if !idx.Dropped() && (!idx.Primary() || desc.IsPhysicalTable()) {
 			lazyAllocAppendIndex(&c.nonDrop, idx, len(c.all))
 		}
-		if idx.IsPartial() {
+		// TODO(ssd): We exclude backfilling indexes from
+		// IsPartial() for the unprincipled reason of not
+		// wanting to modify all of the code that assumes
+		// these are always at least delete-only.
+		if idx.IsPartial() && !idx.Backfilling() {
 			lazyAllocAppendIndex(&c.partial, idx, len(c.all))
 		}
 	}

pkg/sql/catalog/tabledesc/index_test.go

@@ -194,6 +194,13 @@ func TestIndexInterface(t *testing.T) {
 			catalog.ForEachDeletableNonPrimaryIndex,
 			catalog.FindDeletableNonPrimaryIndex,
 		},
+		{
+			"NonPrimaryIndex",
+			indexNames[1:],
+			catalog.TableDescriptor.NonPrimaryIndexes,
+			catalog.ForEachNonPrimaryIndex,
+			catalog.FindNonPrimaryIndex,
+		},
 		{
 			"DeleteOnlyNonPrimaryIndex",
 			[]string{},

pkg/sql/catalog/tabledesc/mutation.go

@@ -62,6 +62,12 @@ func (mm maybeMutation) DeleteOnly() bool {
 	return mm.mutationState == descpb.DescriptorMutation_DELETE_ONLY
 }
 
+// Backfilling returns true iff the table element is a mutation in the
+// backfilling state.
+func (mm maybeMutation) Backfilling() bool {
+	return mm.mutationState == descpb.DescriptorMutation_BACKFILLING
+}
+
 // Adding returns true iff the table element is in an add mutation.
 func (mm maybeMutation) Adding() bool {
 	return mm.mutationDirection == descpb.DescriptorMutation_ADD