Merge cockroachdb#86090 cockroachdb#86146 cockroachdb#86191

86090: CODEOWNERS: stop dev-inf PR review requests for gen file list changes r=rickystewart a=jlinder

Changes to most of the generated pkg/gen/*.bzl files happen for many PRs
and they don't need to be reviewed by dev inf. Let's stop getting PR
review requests for them.

Release note: None
Release justification: non-production code change

86146: insights: call ingester methods by reference r=matthewtodd a=matthewtodd

Fixes the data race observed in cockroachdb#85988. Passing this data structure by
value meant that the `fullHandler` was working with a different object
than the external callers.

Release justification: Category 3: Fixes for high-priority or
high-severity bugs in existing functionality

Release note: None

86191: sql/schemachanger/sctest: run the Backup test over all stages r=ajwerner a=ajwerner

Before this change, we'd only run them over the non-revertible stages.

Release justification: testing only change

Release note: None

Co-authored-by: James H. Linder <[email protected]>
Co-authored-by: Matthew Todd <[email protected]>
Co-authored-by: Andrew Werner <[email protected]>

.github/CODEOWNERS

@@ -188,6 +188,8 @@
 /pkg/ccl/sqlproxyccl/        @cockroachdb/sqlproxy-prs @cockroachdb/server-prs
 
 /pkg/gen/                    @cockroachdb/dev-inf
+/pkg/gen/*.bzl               @cockroachdb/dev-inf-noreview
+/pkg/gen/gen.bzl             @cockroachdb/dev-inf
 
 /pkg/acceptance/             @cockroachdb/sql-experience
 /pkg/base/                   @cockroachdb/unowned @cockroachdb/kv-prs @cockroachdb/server-prs

pkg/sql/schemachanger/sctest/cumulative.go

@@ -327,9 +327,11 @@ func Backup(t *testing.T, dir string, newCluster NewClusterFunc) {
 	testFunc := func(t *testing.T, _ string, _ bool, setup, stmts []parser.Statement) {
 		postCommit, nonRevertible := countStages(t, setup, stmts)
 		n := postCommit + nonRevertible
-
-		t.Logf("test case has %d revertible post-commit stages", n)
-		for i := postCommit; i <= n; i++ {
+		t.Logf(
+			"test case has %d revertible post-commit stages and %d non-revertible"+
+				" post-commit stages", postCommit, nonRevertible,
+		)
+		for i := 0; i <= n; i++ {
 			if !t.Run(
 				fmt.Sprintf("backup/restore stage %d of %d", i, n),
 				func(t *testing.T) { testBackupRestoreCase(t, setup, stmts, i) },
@@ -405,7 +407,11 @@ func Backup(t *testing.T, dir string, newCluster NewClusterFunc) {
 		var backups []backup
 		var done bool
 		var rollbackStage int
-		completedStages := make(map[int]struct{})
+		type stageKey struct {
+			stage    int
+			rollback bool
+		}
+		completedStages := make(map[stageKey]struct{})
 		for i := 0; !done; i++ {
 			// We want to let the stages up to ord continue unscathed. Then, we'll
 			// start taking backups at ord. If ord corresponds to a revertible
@@ -423,10 +429,17 @@ func Backup(t *testing.T, dir string, newCluster NewClusterFunc) {
 			s := <-stageChan
 			// Move the index backwards if we see the same stage repeat due to a txn
 			// retry error for example.
-			if _, ok := completedStages[s.stageIdx]; ok {
+			stage := stageKey{
+				stage:    s.stageIdx,
+				rollback: s.p.InRollback,
+			}
+			if _, ok := completedStages[stage]; ok {
 				i--
+				if stage.rollback {
+					rollbackStage--
+				}
 			}
-			completedStages[s.stageIdx] = struct{}{}
+			completedStages[stage] = struct{}{}
 			shouldFail := ord == i &&
 				s.p.Stages[s.stageIdx].Phase != scop.PostCommitNonRevertiblePhase &&
 				!s.p.InRollback

pkg/sql/sqlstats/insights/ingester.go

@@ -55,7 +55,7 @@ type event struct {
 	statement   *Statement
 }
 
-func (i concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopper) {
+func (i *concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopper) {
 	// This task pulls buffers from the channel and forwards them along to the
 	// underlying Registry.
 	_ = stopper.RunAsyncTask(ctx, "insights-ingester", func(ctx context.Context) {
@@ -86,7 +86,7 @@ func (i concurrentBufferIngester) Start(ctx context.Context, stopper *stop.Stopp
 	})
 }
 
-func (i concurrentBufferIngester) ingest(events *eventBuffer) {
+func (i *concurrentBufferIngester) ingest(events *eventBuffer) {
 	for _, e := range events {
 		// Because an eventBuffer is a fixed-size array, rather than a slice,
 		// we do not know how full it is until we hit a nil entry.
@@ -101,7 +101,7 @@ func (i concurrentBufferIngester) ingest(events *eventBuffer) {
 	}
 }
 
-func (i concurrentBufferIngester) ObserveStatement(
+func (i *concurrentBufferIngester) ObserveStatement(
 	sessionID clusterunique.ID, statement *Statement,
 ) {
 	if !i.enabled() {
@@ -115,7 +115,7 @@ func (i concurrentBufferIngester) ObserveStatement(
 	})
 }
 
-func (i concurrentBufferIngester) ObserveTransaction(
+func (i *concurrentBufferIngester) ObserveTransaction(
 	sessionID clusterunique.ID, transaction *Transaction,
 ) {
 	if !i.enabled() {
@@ -129,13 +129,13 @@ func (i concurrentBufferIngester) ObserveTransaction(
 	})
 }
 
-func (i concurrentBufferIngester) IterateInsights(
+func (i *concurrentBufferIngester) IterateInsights(
 	ctx context.Context, visitor func(context.Context, *Insight),
 ) {
 	i.delegate.IterateInsights(ctx, visitor)
 }
 
-func (i concurrentBufferIngester) enabled() bool {
+func (i *concurrentBufferIngester) enabled() bool {
 	return i.delegate.enabled()
 }
 

pkg/sql/sqlstats/insights/insights.go

@@ -138,5 +138,5 @@ type Registry interface {
 
 // New builds a new Registry.
 func New(st *cluster.Settings, metrics Metrics) Registry {
-	return newRegistry(st, metrics)
+	return newConcurrentBufferIngester(newRegistry(st, metrics))
 }