From b9ddf79a854a46a471a18fb851cbd4309d6849e5 Mon Sep 17 00:00:00 2001 From: Yahor Yuzefovich Date: Wed, 5 Nov 2025 21:33:12 -0800 Subject: [PATCH] sql: remove now-stale growstack for internal executor As of 4a5c503e960d5a9fefb49e4373ef43e37245827c, we now grow stack unconditionally for all async tasks. Some time ago we added an ability to growstack in the async task of the internal executor when used in the LDR, and now that has become redundant, so we remove this special case. Release note: None --- pkg/crosscluster/logical/lww_row_processor.go | 4 ---- pkg/sql/BUILD.bazel | 1 - pkg/sql/internal.go | 10 +--------- pkg/sql/sessiondata/internal.go | 3 --- 4 files changed, 1 insertion(+), 17 deletions(-) diff --git a/pkg/crosscluster/logical/lww_row_processor.go b/pkg/crosscluster/logical/lww_row_processor.go index 36c9efb8e2ac..a3554f08fea9 100644 --- a/pkg/crosscluster/logical/lww_row_processor.go +++ b/pkg/crosscluster/logical/lww_row_processor.go @@ -407,10 +407,6 @@ var ( // Use generic query plans since our queries are extremely simple and // won't benefit from custom plans. PlanCacheMode: &forceGenericPlan, - // We've observed in the CPU profiles that the default goroutine stack - // of the connExecutor goroutine is insufficient for evaluation of the - // ingestion queries, so we grow the stack right away to 32KiB. - GrowStackSize: true, // We don't get any benefits from generating plan gists for internal // queries, so we disable them. DisablePlanGists: true, diff --git a/pkg/sql/BUILD.bazel b/pkg/sql/BUILD.bazel index fbce34068b7b..a3a64b36b17f 100644 --- a/pkg/sql/BUILD.bazel +++ b/pkg/sql/BUILD.bazel @@ -575,7 +575,6 @@ go_library( "//pkg/util/errorutil", "//pkg/util/errorutil/unimplemented", "//pkg/util/fsm", - "//pkg/util/growstack", "//pkg/util/grpcutil", "//pkg/util/grunning", "//pkg/util/hlc", diff --git a/pkg/sql/internal.go b/pkg/sql/internal.go index fdf1ceeffd8a..865c566d519a 100644 --- a/pkg/sql/internal.go +++ b/pkg/sql/internal.go @@ -39,7 +39,6 @@ import ( "github.com/cockroachdb/cockroach/pkg/sql/sqltelemetry" "github.com/cockroachdb/cockroach/pkg/sql/types" "github.com/cockroachdb/cockroach/pkg/util/fsm" - "github.com/cockroachdb/cockroach/pkg/util/growstack" "github.com/cockroachdb/cockroach/pkg/util/log" "github.com/cockroachdb/cockroach/pkg/util/mon" "github.com/cockroachdb/cockroach/pkg/util/retry" @@ -223,7 +222,6 @@ func (ie *InternalExecutor) runWithEx( syncCallback func([]*streamingCommandResult), errCallback func(error), attributeToUser bool, - growStackSize bool, ) error { ex, err := ie.initConnEx(ctx, txn, w, mode, sd, stmtBuf, syncCallback, attributeToUser) if err != nil { @@ -251,11 +249,6 @@ func (ie *InternalExecutor) runWithEx( go func() { defer hdl.Activate(ctx).Release(ctx) defer cleanup(ctx) - // TODO(yuzefovich): benchmark whether we should be growing the - // stack size unconditionally. - if growStackSize { - growstack.Grow() - } if err := ex.run( ctx, ie.mon, @@ -1196,7 +1189,6 @@ func (ie *InternalExecutor) execInternal( txn.SetBufferedWritesEnabled(false) } attributeToUser := sessionDataOverride.AttributeToUser && attributeToUserEnabled.Get(&ie.s.cfg.Settings.SV) - growStackSize := sessionDataOverride.GrowStackSize if !rw.async() && (txn != nil && txn.Type() == kv.RootTxn) { // If the "outer" query uses the RootTxn and the sync result channel is // requested, then we must disable both DistSQL and Streamer to ensure @@ -1308,7 +1300,7 @@ func (ie *InternalExecutor) execInternal( errCallback := func(err error) { _ = rw.addResult(ctx, ieIteratorResult{err: err}) } - err = ie.runWithEx(ctx, opName, txn, rw, mode, sd, stmtBuf, &wg, syncCallback, errCallback, attributeToUser, growStackSize) + err = ie.runWithEx(ctx, opName, txn, rw, mode, sd, stmtBuf, &wg, syncCallback, errCallback, attributeToUser) if err != nil { return nil, err } diff --git a/pkg/sql/sessiondata/internal.go b/pkg/sql/sessiondata/internal.go index 9965b350dcfc..15d3d6110123 100644 --- a/pkg/sql/sessiondata/internal.go +++ b/pkg/sql/sessiondata/internal.go @@ -79,9 +79,6 @@ type InternalExecutorOverride struct { OriginTimestampForLogicalDataReplication hlc.Timestamp // PlanCacheMode, if set, overrides the plan_cache_mode session variable. PlanCacheMode *sessiondatapb.PlanCacheMode - // GrowStackSize, if true, indicates that the connExecutor goroutine stack - // should be grown to 32KiB right away. - GrowStackSize bool // DisablePlanGists, if true, overrides the disable_plan_gists session var. DisablePlanGists bool // BufferedWritesEnabled, if set, controls whether the buffered writes KV transaction