Merge #149812 #150256

149812: sql: disable vector index backfill in the legacy schema changer r=mw5h a=mw5h

Through an oversight, the legacy schema changer never received a copy of
the vector manager, required for backfilling vector indexes. In
practice, we always use the declarative schema changer for creating
vector indexes, so this wasn't caught in QA. Sentry caught the
oversight.

Completely fixing this problem is more complicated than what we have
time to do in 25.3, so for now the plan is to disable backfilling vector
indices with the legacy schema changer, which should be a corner case to
begin with.

Informs: #149236
Release note (bug fix): Attempting to create a vector index with the
legacy schema changer will now fail gracefully instead of crashing the
node.

150256: server: fixes test flaps due to drains timing out r=[kyle-a-wong] a=MattWhelan

Use the SucceedSoonDuration rather than a static 30s timeout. This is responsive to various test parameters.

Fixes: #149515, #149534

Release note: None

Co-authored-by: Matt White <[email protected]>
Co-authored-by: Matt Whelan <[email protected]>

Author: 3 people

pkg/server/api_v2_test.go

@@ -535,7 +535,7 @@ func TestCheckRestartSafe_Integration(t *testing.T) {
 }
 
 func drain(ctx context.Context, ts1 serverutils.TestServerInterface, t *testing.T) error {
-	timeoutCtx, cancel := context.WithTimeout(ctx, 30*time.Second)
+	timeoutCtx, cancel := context.WithTimeout(ctx, testutils.SucceedsSoonDuration())
 	defer cancel()
 
 	err := ts1.DrainClients(ctx)

pkg/sql/backfill.go

@@ -2917,7 +2917,12 @@ func indexBackfillInTxn(
 
 	var backfiller backfill.IndexBackfiller
 	if err := backfiller.InitForLocalUse(
-		ctx, evalCtx, semaCtx, tableDesc, indexBackfillerMon,
+		ctx,
+		evalCtx,
+		semaCtx,
+		tableDesc,
+		indexBackfillerMon,
+		evalCtx.Planner.ExecutorConfig().(*ExecutorConfig).VecIndexManager,
 	); err != nil {
 		return err
 	}

pkg/sql/backfill/BUILD.bazel

@@ -49,6 +49,7 @@ go_library(
         "//pkg/sql/vecindex/vecstore/vecstorepb",
         "//pkg/util/admission/admissionpb",
         "//pkg/util/ctxgroup",
+        "//pkg/util/errorutil/unimplemented",
         "//pkg/util/hlc",
         "//pkg/util/intsets",
         "//pkg/util/log",

pkg/sql/backfill/backfill.go

@@ -40,6 +40,7 @@ import (
 	"github.com/cockroachdb/cockroach/pkg/sql/vecindex/vecencoding"
 	"github.com/cockroachdb/cockroach/pkg/sql/vecindex/vecstore"
 	"github.com/cockroachdb/cockroach/pkg/sql/vecindex/vecstore/vecstorepb"
+	"github.com/cockroachdb/cockroach/pkg/util/errorutil/unimplemented"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
 	"github.com/cockroachdb/cockroach/pkg/util/mon"
 	"github.com/cockroachdb/cockroach/pkg/util/syncutil"
@@ -605,6 +606,7 @@ func (ib *IndexBackfiller) InitForLocalUse(
 	semaCtx *tree.SemaContext,
 	desc catalog.TableDescriptor,
 	mon *mon.BytesMonitor,
+	vecIndexManager *vecindex.Manager,
 ) (retErr error) {
 	if mon == nil {
 		return errors.AssertionFailedf("memory monitor must be provided")
@@ -616,7 +618,8 @@ func (ib *IndexBackfiller) InitForLocalUse(
 	}()
 
 	// Initialize ib.added.
-	if err := ib.initIndexes(ctx, evalCtx, desc, nil /* allowList */, 0 /*sourceIndex*/, nil); err != nil {
+	// TODO(150163): Pass vecIndexManager once vector index build is supported with the legacy schema changer.
+	if err := ib.initIndexes(ctx, evalCtx, desc, nil /* allowList */, 0 /*sourceIndex*/, nil /*vecIndexManager*/); err != nil {
 		return err
 	}
 
@@ -921,6 +924,13 @@ func (ib *IndexBackfiller) initIndexes(
 			continue
 		}
 
+		if vecIndexManager == nil {
+			return unimplemented.NewWithIssue(
+				150163,
+				"vector index build not supported with the legacy schema changer",
+			)
+		}
+
 		if ib.VectorIndexes == nil {
 			ib.VectorIndexes = make(map[descpb.IndexID]VectorIndexHelper)
 		}

pkg/sql/logictest/testdata/logic_test/vector_index

@@ -1141,3 +1141,25 @@ statement ok
 DROP TABLE composite_pk
 
 subtest end
+
+subtest test_backfill_149236
+
+statement ok
+set autocommit_before_ddl=off;
+
+statement ok
+BEGIN;
+
+statement ok
+CREATE TABLE test_backfill_149236 (a INT PRIMARY KEY, b INT, vec1 VECTOR(3));
+
+statement error pgcode 0A000 vector index build not supported with the legacy schema changer
+CREATE VECTOR INDEX idx ON test_backfill_149236 (vec1);
+
+statement ok
+COMMIT;
+
+statement ok
+SET autocommit_before_ddl=on;
+
+subtest end