From 9cdc63a66a9135be3c1a913b3dff0a903fba02a4 Mon Sep 17 00:00:00 2001 From: Mufeez Amjad Date: Mon, 24 Jan 2022 17:56:54 -0500 Subject: [PATCH 1/4] scripts/gceworker.sh: remove extra s in sleep command Release note: None --- scripts/gceworker.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/scripts/gceworker.sh b/scripts/gceworker.sh index 5a8b0aec5324..7841c3d44ac3 100755 --- a/scripts/gceworker.sh +++ b/scripts/gceworker.sh @@ -48,7 +48,7 @@ case "${cmd}" in gcloud compute firewall-rules create "${NAME}-mosh" --allow udp:60000-61000 # wait a bit to let gcloud create the instance before retrying - sleep 30s + sleep 30 # Retry while vm and sshd start up. retry gcloud compute ssh "${NAME}" --command=true From ec02421b9c2325ae8ffdf449d1c5cad951281d7f Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Mon, 24 Jan 2022 17:10:08 -0600 Subject: [PATCH 2/4] ci: add script for `compose` nightly, `dev` support Closes #67151. Closes #70893. Release note: None --- build/teamcity/cockroach/nightlies/compose.sh | 18 +++++ pkg/build/bazel/non_bazel.go | 5 ++ pkg/cmd/dev/BUILD.bazel | 1 + pkg/cmd/dev/acceptance.go | 1 - pkg/cmd/dev/compose.go | 59 +++++++++++++++++ pkg/cmd/dev/dev.go | 1 + pkg/compose/BUILD.bazel | 6 +- pkg/compose/compare/compare/BUILD.bazel | 1 + pkg/compose/compare/docker-compose.yml | 10 +-- pkg/compose/compose_test.go | 66 ++++++++++++++++++- 10 files changed, 158 insertions(+), 10 deletions(-) create mode 100755 build/teamcity/cockroach/nightlies/compose.sh create mode 100644 pkg/cmd/dev/compose.go diff --git a/build/teamcity/cockroach/nightlies/compose.sh b/build/teamcity/cockroach/nightlies/compose.sh new file mode 100755 index 000000000000..4039397bd3c3 --- /dev/null +++ b/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" diff --git a/pkg/build/bazel/non_bazel.go b/pkg/build/bazel/non_bazel.go index 78115eb7e1c8..534e2683fbaa 100644 --- a/pkg/build/bazel/non_bazel.go +++ b/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") diff --git a/pkg/cmd/dev/BUILD.bazel b/pkg/cmd/dev/BUILD.bazel index ea8b271a5f47..929757ed6992 100644 --- a/pkg/cmd/dev/BUILD.bazel +++ b/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", diff --git a/pkg/cmd/dev/acceptance.go b/pkg/cmd/dev/acceptance.go index 1a1e957a9577..f8537ddd8eaa 100644 --- a/pkg/cmd/dev/acceptance.go +++ b/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) diff --git a/pkg/cmd/dev/compose.go b/pkg/cmd/dev/compose.go new file mode 100644 index 000000000000..d7e096e618b9 --- /dev/null +++ b/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...) +} diff --git a/pkg/cmd/dev/dev.go b/pkg/cmd/dev/dev.go index 228a05de23d5..cd462a3a18cd 100644 --- a/pkg/cmd/dev/dev.go +++ b/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), diff --git a/pkg/compose/BUILD.bazel b/pkg/compose/BUILD.bazel index 09d763a23e23..c08c3b42cf82 100644 --- a/pkg/compose/BUILD.bazel +++ b/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"], diff --git a/pkg/compose/compare/compare/BUILD.bazel b/pkg/compose/compare/compare/BUILD.bazel index 7b7ea49ef64e..a941ec287a86 100644 --- a/pkg/compose/compare/compare/BUILD.bazel +++ b/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", diff --git a/pkg/compose/compare/docker-compose.yml b/pkg/compose/compare/docker-compose.yml index 89e8c2907348..76b27eb56b46 100644 --- a/pkg/compose/compare/docker-compose.yml +++ b/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" diff --git a/pkg/compose/compose_test.go b/pkg/compose/compose_test.go index a34711334e9f..02eb5e0e480b 100644 --- a/pkg/compose/compose_test.go +++ b/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 { From f81a7e406ecc629eea3bf37533a8b8156ef7c20f Mon Sep 17 00:00:00 2001 From: Steven Danna Date: Mon, 1 Nov 2021 14:00:12 +0000 Subject: [PATCH 3/4] sql: add new BACKFILLING mutation state 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` and `(TableDescriptor).NonDropIndexes`. Release note: None --- pkg/sql/catalog/descpb/structured.proto | 13 +++++ pkg/sql/catalog/descriptor.go | 12 ++++- pkg/sql/catalog/table_elements.go | 16 ++++++ pkg/sql/catalog/tabledesc/index.go | 15 +++++- pkg/sql/catalog/tabledesc/index_test.go | 7 +++ pkg/sql/catalog/tabledesc/mutation.go | 6 +++ pkg/sql/catalog/tabledesc/structured.go | 2 +- pkg/sql/catalog/tabledesc/table_desc.go | 16 ++++-- pkg/sql/descriptor_mutation_test.go | 66 ++++++++++++++++++------- pkg/sql/opt_catalog.go | 2 +- 10 files changed, 128 insertions(+), 27 deletions(-) diff --git a/pkg/sql/catalog/descpb/structured.proto b/pkg/sql/catalog/descpb/structured.proto index 491835e2c6df..7a118e6bff4e 100644 --- a/pkg/sql/catalog/descpb/structured.proto +++ b/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]; diff --git a/pkg/sql/catalog/descriptor.go b/pkg/sql/catalog/descriptor.go index 58e9f93562f0..f5a8f1aff6c2 100644 --- a/pkg/sql/catalog/descriptor.go +++ b/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 diff --git a/pkg/sql/catalog/table_elements.go b/pkg/sql/catalog/table_elements.go index ea97bbaa1c14..f8ec82b129d7 100644 --- a/pkg/sql/catalog/table_elements.go +++ b/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 { diff --git a/pkg/sql/catalog/tabledesc/index.go b/pkg/sql/catalog/tabledesc/index.go index 8734792be6ac..67ff328a275f 100644 --- a/pkg/sql/catalog/tabledesc/index.go +++ b/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)) } } diff --git a/pkg/sql/catalog/tabledesc/index_test.go b/pkg/sql/catalog/tabledesc/index_test.go index 8b4bf59572f9..0cdc7e49771a 100644 --- a/pkg/sql/catalog/tabledesc/index_test.go +++ b/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{}, diff --git a/pkg/sql/catalog/tabledesc/mutation.go b/pkg/sql/catalog/tabledesc/mutation.go index b0ba13c09000..39fd4d350788 100644 --- a/pkg/sql/catalog/tabledesc/mutation.go +++ b/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 diff --git a/pkg/sql/catalog/tabledesc/structured.go b/pkg/sql/catalog/tabledesc/structured.go index aac40d730fc7..d65eb195880c 100644 --- a/pkg/sql/catalog/tabledesc/structured.go +++ b/pkg/sql/catalog/tabledesc/structured.go @@ -708,7 +708,7 @@ func (desc *Mutable) allocateIndexIDs(columnNames map[string]descpb.ColumnID) er } // Assign names to unnamed indexes. - err := catalog.ForEachDeletableNonPrimaryIndex(desc, func(idx catalog.Index) error { + err := catalog.ForEachNonPrimaryIndex(desc, func(idx catalog.Index) error { if len(idx.GetName()) == 0 { name, err := BuildIndexName(desc, idx.IndexDesc()) if err != nil { diff --git a/pkg/sql/catalog/tabledesc/table_desc.go b/pkg/sql/catalog/tabledesc/table_desc.go index ea65b8579aef..1f58a60e8062 100644 --- a/pkg/sql/catalog/tabledesc/table_desc.go +++ b/pkg/sql/catalog/tabledesc/table_desc.go @@ -288,6 +288,13 @@ func (desc *wrapper) PartialIndexes() []catalog.Index { return desc.getExistingOrNewIndexCache().partial } +// NonPrimaryIndexes returns a slice of all non-primary indexes, in +// their canonical order. This is equivalent to taking the slice +// produced by AllIndexes and removing the primary index. +func (desc *wrapper) NonPrimaryIndexes() []catalog.Index { + return desc.getExistingOrNewIndexCache().nonPrimary +} + // PublicNonPrimaryIndexes returns a slice of all active secondary indexes, // in their canonical order. This is equivalent to the Indexes array in the // proto. @@ -304,10 +311,11 @@ func (desc *wrapper) WritableNonPrimaryIndexes() []catalog.Index { return desc.getExistingOrNewIndexCache().writableNonPrimary } -// 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. +// 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 and backfilling indexes. func (desc *wrapper) DeletableNonPrimaryIndexes() []catalog.Index { return desc.getExistingOrNewIndexCache().deletableNonPrimary } diff --git a/pkg/sql/descriptor_mutation_test.go b/pkg/sql/descriptor_mutation_test.go index 6720af68e054..d1da14411bde 100644 --- a/pkg/sql/descriptor_mutation_test.go +++ b/pkg/sql/descriptor_mutation_test.go @@ -529,8 +529,11 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); indexQuery := `SELECT v FROM t.test@foo` for _, useUpsert := range []bool{true, false} { // See the effect of the operations depending on the state. - for _, state := range []descpb.DescriptorMutation_State{descpb.DescriptorMutation_DELETE_ONLY, - descpb.DescriptorMutation_DELETE_AND_WRITE_ONLY} { + for _, state := range []descpb.DescriptorMutation_State{ + descpb.DescriptorMutation_DELETE_ONLY, + descpb.DescriptorMutation_DELETE_AND_WRITE_ONLY, + descpb.DescriptorMutation_BACKFILLING, + } { // Init table with some entries. if _, err := sqlDB.Exec(`TRUNCATE TABLE t.test`); err != nil { t.Fatal(err) @@ -573,12 +576,14 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); // Make index "foo" live so that we can read it. mTest.makeMutationsActive(ctx) - if state == descpb.DescriptorMutation_DELETE_ONLY { + switch state { + case descpb.DescriptorMutation_DELETE_ONLY, descpb.DescriptorMutation_BACKFILLING: // "x" didn't get added to the index. mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}, {"z"}}) - } else { + default: // "x" got added to the index. mTest.CheckQueryResults(t, indexQuery, [][]string{{"x"}, {"y"}, {"z"}}) + } // Make "foo" a mutation. @@ -597,11 +602,15 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); // Make index "foo" live so that we can read it. mTest.makeMutationsActive(ctx) - if state == descpb.DescriptorMutation_DELETE_ONLY { + switch state { + case descpb.DescriptorMutation_BACKFILLING: + // Update results in no modifications to index. + mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}, {"z"}}) + case descpb.DescriptorMutation_DELETE_ONLY: // updating "x" -> "w" will result in "x" being deleted from the index. // updating "z" -> "z" results in "z" being deleted from the index. mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}}) - } else { + default: // updating "x" -> "w" results in the index updating from "x" -> "w", // updating "z" -> "z" is a noop on the index. mTest.CheckQueryResults(t, indexQuery, [][]string{{"w"}, {"y"}, {"z"}}) @@ -618,9 +627,13 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); // Updating the primary key for a row when we're in delete-only won't // create a new index entry, and will delete the old one. Otherwise it'll // create a new entry and delete the old one. - if state == descpb.DescriptorMutation_DELETE_ONLY { + switch state { + case descpb.DescriptorMutation_BACKFILLING: + // Update results in no modifications to index. + mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}, {"z"}}) + case descpb.DescriptorMutation_DELETE_ONLY: mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}}) - } else { + default: mTest.CheckQueryResults(t, indexQuery, [][]string{{"w"}, {"y"}, {"z"}}) } @@ -633,9 +646,13 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR, INDEX foo (v)); // Make index "foo" live so that we can read it. mTest.makeMutationsActive(ctx) // Deleting row "b" deletes "y" from the index. - if state == descpb.DescriptorMutation_DELETE_ONLY { + switch state { + case descpb.DescriptorMutation_BACKFILLING: + // Update results in no modifications to index. + mTest.CheckQueryResults(t, indexQuery, [][]string{{"y"}, {"z"}}) + case descpb.DescriptorMutation_DELETE_ONLY: mTest.CheckQueryResults(t, indexQuery, [][]string{}) - } else { + default: mTest.CheckQueryResults(t, indexQuery, [][]string{{"w"}, {"z"}}) } } @@ -698,6 +715,7 @@ CREATE INDEX allidx ON t.test (k, v); for _, idxState := range []descpb.DescriptorMutation_State{ descpb.DescriptorMutation_DELETE_ONLY, descpb.DescriptorMutation_DELETE_AND_WRITE_ONLY, + descpb.DescriptorMutation_BACKFILLING, } { // Ignore the impossible column in DELETE_ONLY state while index // is in the DELETE_AND_WRITE_ONLY state. @@ -748,10 +766,14 @@ CREATE INDEX allidx ON t.test (k, v); mTest.makeMutationsActive(ctx) // column "i" has no entry. mTest.CheckQueryResults(t, starQuery, [][]string{{"a", "z", "q"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) - if idxState == descpb.DescriptorMutation_DELETE_ONLY { + switch idxState { + case descpb.DescriptorMutation_DELETE_ONLY: // No index entry for row "c" mTest.CheckQueryResults(t, indexQuery, [][]string{{"q"}, {"r"}}) - } else { + case descpb.DescriptorMutation_BACKFILLING: + // No index modification + mTest.CheckQueryResults(t, indexQuery, [][]string{{"q"}, {"r"}}) + default: // Index entry for row "c" mTest.CheckQueryResults(t, indexQuery, [][]string{{"NULL"}, {"q"}, {"r"}}) } @@ -809,10 +831,14 @@ CREATE INDEX allidx ON t.test (k, v); mTest.CheckQueryResults(t, starQuery, [][]string{{"a", "u", "NULL"}, {"b", "y", "r"}, {"c", "x", "NULL"}}) } - if idxState == descpb.DescriptorMutation_DELETE_ONLY { + switch idxState { + case descpb.DescriptorMutation_DELETE_ONLY: // Index entry for row "a" is deleted. mTest.CheckQueryResults(t, indexQuery, [][]string{{"r"}}) - } else { + case descpb.DescriptorMutation_BACKFILLING: + // No index modification + mTest.CheckQueryResults(t, indexQuery, [][]string{{"q"}, {"r"}}) + default: // Index "foo" has NULL "i" value for row "a". mTest.CheckQueryResults(t, indexQuery, [][]string{{"NULL"}, {"NULL"}, {"r"}}) } @@ -841,10 +867,15 @@ CREATE INDEX allidx ON t.test (k, v); numKVs++ } - if idxState == descpb.DescriptorMutation_DELETE_ONLY { + switch idxState { + case descpb.DescriptorMutation_DELETE_ONLY: // Index entry for row "a" is deleted. mTest.CheckQueryResults(t, indexQuery, [][]string{}) - } else { + // No index modification + case descpb.DescriptorMutation_BACKFILLING: + mTest.CheckQueryResults(t, indexQuery, [][]string{{"q"}, {"r"}}) + numKVs += 2 + default: // Index entry for row "a" is deleted. if state == descpb.DescriptorMutation_DELETE_ONLY { mTest.CheckQueryResults(t, indexQuery, [][]string{{"NULL"}, {"q"}}) @@ -854,7 +885,6 @@ CREATE INDEX allidx ON t.test (k, v); // We have two index values. numKVs += 2 } - // Check that there are no hidden KV values for row "b", and column // "i" for row "b" was deleted. Also check that the index values are // all accounted for. @@ -1182,6 +1212,8 @@ CREATE TABLE t.test (k CHAR PRIMARY KEY, v CHAR UNIQUE); actualState = descpb.DescriptorMutation_DELETE_AND_WRITE_ONLY } else if m.DeleteOnly() { actualState = descpb.DescriptorMutation_DELETE_ONLY + } else if m.Backfilling() { + actualState = descpb.DescriptorMutation_BACKFILLING } if state := expected[i].state; actualState != state { t.Errorf("%d entry: state %s, expected %s", i, actualState, state) diff --git a/pkg/sql/opt_catalog.go b/pkg/sql/opt_catalog.go index de3ae8ab0f84..967570f858f7 100644 --- a/pkg/sql/opt_catalog.go +++ b/pkg/sql/opt_catalog.go @@ -1067,7 +1067,7 @@ func (ot *optTable) WritableIndexCount() int { // DeletableIndexCount is part of the cat.Table interface. func (ot *optTable) DeletableIndexCount() int { // Primary index is always present, so count is always >= 1. - return len(ot.desc.AllIndexes()) + return len(ot.desc.DeletableNonPrimaryIndexes()) + 1 } // Index is part of the cat.Table interface. From 79a429efb8c349a54883fd4aa35ece5f89ceb941 Mon Sep 17 00:00:00 2001 From: Ricky Stewart Date: Wed, 26 Jan 2022 11:11:04 -0600 Subject: [PATCH 4/4] distsql: bump size of test This has timed out in CI. Release note: None --- pkg/sql/distsql/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pkg/sql/distsql/BUILD.bazel b/pkg/sql/distsql/BUILD.bazel index ce141e42fef2..6cabc87c53b3 100644 --- a/pkg/sql/distsql/BUILD.bazel +++ b/pkg/sql/distsql/BUILD.bazel @@ -34,7 +34,7 @@ go_library( go_test( name = "distsql_test", - size = "medium", + size = "large", srcs = [ "columnar_operators_test.go", "columnar_utils_test.go",