Merge #159205

159205: util/admission: add runtime.Yield to pacer.Pace() calls r=dt a=dt

Including for otherwise disabled pacers, such as in features where elastic CPU limiting has been kept behind a setting.

Release note (performance change): Various background tasks and jobs now more actively yield to foreground work when it is waiting to run.
Epic: CRDB-37540.

Co-authored-by: David Taylor <[email protected]>

Author: craig[bot]

pkg/kv/bulk/cpu_pacer.go

@@ -25,11 +25,23 @@ var cpuPacerRequestDuration = settings.RegisterDurationSetting(
 	50*time.Millisecond,
 )
 
+var yieldIfNoPacer = settings.RegisterBoolSetting(
+	settings.ApplicationLevel,
+	"bulkio.elastic_cpu_control.always_yield.enabled",
+	"if true, yield the CPU as needed even when time-based elastic pacing is not enabled",
+	true,
+)
+
 // NewCPUPacer creates a new AC pacer for SST batcher. It may return an empty
 // Pacer which noops if pacing is disabled or its arguments are nil.
 func NewCPUPacer(ctx context.Context, db *kv.DB, setting *settings.BoolSetting) *admission.Pacer {
 	if db == nil || db.AdmissionPacerFactory == nil || !setting.Get(db.SettingsValues()) {
 		log.Dev.Infof(ctx, "admission control is not configured to pace bulk ingestion")
+
+		if db != nil && yieldIfNoPacer.Get(db.SettingsValues()) {
+			// Return a Pacer that just yields.
+			return &admission.Pacer{Yield: true}
+		}
 		return nil
 	}
 	tenantID, ok := roachpb.ClientTenantFromContext(ctx)

pkg/util/admission/elastic_cpu_grant_coordinator.go

@@ -8,6 +8,7 @@ package admission
 import (
 	"time"
 
+	"github.com/cockroachdb/cockroach/pkg/settings"
 	"github.com/cockroachdb/cockroach/pkg/settings/cluster"
 	"github.com/cockroachdb/cockroach/pkg/util/admission/admissionpb"
 	"github.com/cockroachdb/cockroach/pkg/util/log"
@@ -83,14 +84,22 @@ func (e *ElasticCPUGrantCoordinator) tryGrant() {
 	e.elasticCPUGranter.tryGrant()
 }
 
+var yieldInPacer = settings.RegisterBoolSetting(
+	settings.ApplicationLevel,
+	"admission.elastic_cpu.yield_in_pacer.enabled",
+	"when true, time-based elastic CPU pacing additionally yields CPU as-needed according to the scheduler",
+	true,
+)
+
 // NewPacer implements the PacerMaker interface.
 func (e *ElasticCPUGrantCoordinator) NewPacer(unit time.Duration, wi WorkInfo) *Pacer {
 	if e == nil {
 		return nil
 	}
 	return &Pacer{
-		unit: unit,
-		wi:   wi,
-		wq:   e.ElasticCPUWorkQueue,
+		unit:  unit,
+		wi:    wi,
+		wq:    e.ElasticCPUWorkQueue,
+		Yield: yieldInPacer.Get(&e.ElasticCPUWorkQueue.settings.SV),
 	}
 }

pkg/util/admission/pacer.go

@@ -7,6 +7,7 @@ package admission
 
 import (
 	"context"
+	"runtime"
 	"time"
 )
 
@@ -21,6 +22,13 @@ type Pacer struct {
 	wq   *ElasticCPUWorkQueue
 
 	cur *ElasticCPUWorkHandle
+
+	// Yield, if true, indicates that the Pacer should runtime.Yield() in each
+	// Pace() call, even if the call is otherwise a no-op due to wq being nil i.e.
+	// when time-based pacing is not enabled. Eventually this might just become
+	// the default behavior for nil *Pacer, but the bool allows it to be opt-in
+	// initially.
+	Yield bool
 }
 
 // Pace will block as needed to pace work that calls it. It is
@@ -39,6 +47,14 @@ func (p *Pacer) Pace(ctx context.Context) (readmitted bool, err error) {
 		return false, nil
 	}
 
+	if p.Yield {
+		runtime.Yield()
+	}
+
+	if p.wq == nil {
+		return false, nil
+	}
+
 	if overLimit, _ := p.cur.OverLimit(); overLimit {
 		p.wq.AdmittedWorkDone(p.cur)
 		p.cur = nil