Merge cockroachdb#130397 cockroachdb#130588

130397: stop: annotate tasks with `trace.Region` r=nvanbenschoten a=tbg

From the docs[^1]:

> A region is for logging a time interval during a goroutine's
> execution. By definition, a region starts and ends in the same
> goroutine. Regions can be nested to represent subintervals.

`StartRegion` is a no-op when no execution trace is ongoing[^2]. It does
make one small allocation otherwise, which is a little silly since a
better API could have avoided it.

The result of this change is that execution traces are easier
to inspect visually, as many goroutines in the system will
now state their purpose (the task name) as opposed to looking
like yet another opaque "(*Stopper).RunAsyncTaskEx.func2".

![](https://github.com/user-attachments/assets/f6c20a1a-01da-4ef1-b0f8-edea3132e21c)



Sadly, goroutines that tagged their region before the execution
trace begins will not reflect their region. So, for example, a
`(*raftSchedulerShard).worker`, despite running inside of a task,
will not have a region attached to it (since the `StartRegion`
call happened at server start,  before the execution trace began).

[^1]: https://pkg.go.dev/runtime/trace#hdr-User_annotation
[^2]: https://cs.opensource.google/go/go/+/refs/tags/go1.23.1:src/runtime/trace/annotation.go;l=144-159

Epic: none
Release note: None


130588: opt: remove unnecessary pointer indirection r=mgartner a=mgartner

This commit removes unnecessary pointer indirection in the join order
builder.

Epic: None

Release note: None


Co-authored-by: Tobias Grieger <[email protected]>
Co-authored-by: Marcus Gartner <[email protected]>

pkg/sql/opt/xform/join_order_builder.go

@@ -709,13 +709,13 @@ func (jb *JoinOrderBuilder) addJoins(s1, s2 vertexSet) {
 func (jb *JoinOrderBuilder) makeInnerEdge(op *operator, filters memo.FiltersExpr) {
 	if len(filters) == 0 {
 		// This is a cross join. Create a single edge for the empty FiltersExpr.
-		jb.edges = append(jb.edges, *jb.makeEdge(op, filters))
+		jb.edges = append(jb.edges, jb.makeEdge(op, filters))
 		jb.innerEdges.Add(len(jb.edges) - 1)
 		return
 	}
 	for i := range filters {
 		// Create an edge for each conjunct.
-		jb.edges = append(jb.edges, *jb.makeEdge(op, filters[i:i+1]))
+		jb.edges = append(jb.edges, jb.makeEdge(op, filters[i:i+1]))
 		jb.innerEdges.Add(len(jb.edges) - 1)
 	}
 }
@@ -724,7 +724,7 @@ func (jb *JoinOrderBuilder) makeInnerEdge(op *operator, filters memo.FiltersExpr
 // join. For any given non-inner join, exactly one edge is constructed.
 func (jb *JoinOrderBuilder) makeNonInnerEdge(op *operator, filters memo.FiltersExpr) {
 	// Always create a single edge from a non-inner join.
-	jb.edges = append(jb.edges, *jb.makeEdge(op, filters))
+	jb.edges = append(jb.edges, jb.makeEdge(op, filters))
 	jb.nonInnerEdges.Add(len(jb.edges) - 1)
 }
 
@@ -776,13 +776,13 @@ func (jb *JoinOrderBuilder) makeTransitiveEdge(col1, col2 opt.ColumnID) {
 	filters := memo.FiltersExpr{jb.f.ConstructFiltersItem(condition)}
 
 	// Add the edge to the join graph.
-	jb.edges = append(jb.edges, *jb.makeEdge(op, filters))
+	jb.edges = append(jb.edges, jb.makeEdge(op, filters))
 	jb.innerEdges.Add(len(jb.edges) - 1)
 }
 
 // makeEdge returns a new edge given an operator and set of filters.
-func (jb *JoinOrderBuilder) makeEdge(op *operator, filters memo.FiltersExpr) (e *edge) {
-	e = &edge{op: op, filters: filters}
+func (jb *JoinOrderBuilder) makeEdge(op *operator, filters memo.FiltersExpr) (e edge) {
+	e = edge{op: op, filters: filters}
 	e.calcNullRejectedRels(jb)
 	e.calcSES(jb)
 	e.calcTES(jb.edges)

pkg/util/stop/stopper.go

@@ -15,6 +15,7 @@ import (
 	"fmt"
 	"net/http"
 	"runtime/debug"
+	"runtime/trace"
 	"sync/atomic"
 	"testing"
 	"time"
@@ -315,11 +316,27 @@ func (s *Stopper) RunTask(ctx context.Context, taskName string, f func(context.C
 	// Call f.
 	defer s.recover(ctx)
 	defer s.runPostlude()
+	defer s.startRegion(ctx, taskName).End()
 
 	f(ctx)
 	return nil
 }
 
+type region interface {
+	End()
+}
+
+type noopRegion struct{}
+
+func (n noopRegion) End() {}
+
+func (s *Stopper) startRegion(ctx context.Context, taskName string) region {
+	if !trace.IsEnabled() {
+		return noopRegion{}
+	}
+	return trace.StartRegion(ctx, taskName)
+}
+
 // RunTaskWithErr is like RunTask(), but takes in a callback that can return an
 // error. The error is returned to the caller.
 func (s *Stopper) RunTaskWithErr(
@@ -332,6 +349,7 @@ func (s *Stopper) RunTaskWithErr(
 	// Call f.
 	defer s.recover(ctx)
 	defer s.runPostlude()
+	defer s.startRegion(ctx, taskName).End()
 
 	return f(ctx)
 }
@@ -472,8 +490,9 @@ func (s *Stopper) RunAsyncTaskEx(ctx context.Context, opt TaskOpts, f func(conte
 
 	// Call f on another goroutine.
 	taskStarted = true // Another goroutine now takes ownership of the alloc, if any.
-	go func() {
+	go func(taskName string) {
 		defer s.runPostlude()
+		defer s.startRegion(ctx, taskName).End()
 		defer sp.Finish()
 		defer s.recover(ctx)
 		if alloc != nil {
@@ -482,7 +501,7 @@ func (s *Stopper) RunAsyncTaskEx(ctx context.Context, opt TaskOpts, f func(conte
 
 		sp.UpdateGoroutineIDToCurrent()
 		f(ctx)
-	}()
+	}(opt.TaskName)
 	return nil
 }