diff --git a/pkg/cmd/roachtest/tests/kv.go b/pkg/cmd/roachtest/tests/kv.go index cf6ca81a1d58..c6d46fadeba1 100644 --- a/pkg/cmd/roachtest/tests/kv.go +++ b/pkg/cmd/roachtest/tests/kv.go @@ -757,6 +757,7 @@ func registerKVScalability(r registry.Registry) { const maxPerNodeConcurrency = 64 for i := nodes; i <= nodes*maxPerNodeConcurrency; i += nodes { + i := i // capture loop variable c.Wipe(ctx, c.Range(1, nodes)) c.Start(ctx, t.L(), option.DefaultStartOpts(), install.MakeClusterSettings(), c.Range(1, nodes)) diff --git a/pkg/testutils/lint/passes/loopvarcapture/BUILD.bazel b/pkg/testutils/lint/passes/loopvarcapture/BUILD.bazel index 42b1a18bb93e..148fb78586c6 100644 --- a/pkg/testutils/lint/passes/loopvarcapture/BUILD.bazel +++ b/pkg/testutils/lint/passes/loopvarcapture/BUILD.bazel @@ -25,6 +25,18 @@ go_test( data = glob(["testdata/**"]) + [ "@go_sdk//:files", ], + # N.B. We must disable CGO owing to a rather obscure failure [1]. + # It's surmised that the root cause is the missing CGO metadata + # inside the test's runfiles directory. By disabling CGO, we're + # allowing the go type checker to create a dummy object [2] without + # causing a (type) resolution error. + # + # [1] https://github.com/golang/go/issues/36547 + # [2] https://github.com/golang/tools/blob/master/go/packages/packages.go#L1056-L1064 + # + env = { + "CGO_ENABLED": "0", + }, deps = [ ":loopvarcapture", "//pkg/build/bazel", diff --git a/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture.go b/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture.go index 4200abce9848..58a74864e1c0 100644 --- a/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture.go +++ b/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture.go @@ -405,9 +405,8 @@ func (v *Visitor) visitLoopClosure(closure *ast.FuncLit) { // whether that call is being made to one of the functions in the // GoRoutineFunctions slice. func (v *Visitor) isGoRoutineFunction(call *ast.CallExpr) bool { - callee := typeutil.StaticCallee(v.pass.TypesInfo, call) - // call to a builtin - if callee == nil { + callee, ok := typeutil.Callee(v.pass.TypesInfo, call).(*types.Func) + if !ok { return false } pkg := callee.Pkg() diff --git a/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture_test.go b/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture_test.go index db8bba3c0778..2304b9e4598e 100644 --- a/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture_test.go +++ b/pkg/testutils/lint/passes/loopvarcapture/loopvarcapture_test.go @@ -24,6 +24,7 @@ var extraGoRoutineFunctions = []loopvarcapture.Function{ {Pkg: "example.org/concurrency", Type: "Group", Name: "Go"}, // test non-pointer receiver {Pkg: "example.org/concurrency", Name: "Go"}, // test a package-level function {Pkg: "example.org/concurrency", Name: "GoWithError"}, // test a function with a return value + {Pkg: "p", Type: "Monitor", Name: "Go"}, // test an interface method } func init() { @@ -35,17 +36,16 @@ func init() { func TestAnalyzer(t *testing.T) { skip.UnderStress(t) - // The test fails unless RunDespiteErrors is set to true. - // This is not fully understood, something to do with missing metadata - // for the "C" pseudo-package. - // See comments on #84867 and https://github.com/golang/go/issues/36547. testAnalyzer := *loopvarcapture.Analyzer - testAnalyzer.RunDespiteErrors = true originalGoRoutineFunctions := loopvarcapture.GoRoutineFunctions loopvarcapture.GoRoutineFunctions = append(originalGoRoutineFunctions, extraGoRoutineFunctions...) defer func() { loopvarcapture.GoRoutineFunctions = originalGoRoutineFunctions }() testdata := datapathutils.TestDataPath(t) analysistest.TestData = func() string { return testdata } - analysistest.Run(t, testdata, &testAnalyzer, "p") + results := analysistest.Run(t, testdata, &testAnalyzer, "p") + // Perform a sanity check--we expect analysis results. + if len(results) == 0 { + t.Fatal("analysis results are empty") + } } diff --git a/pkg/testutils/lint/passes/loopvarcapture/testdata/src/p/p.go b/pkg/testutils/lint/passes/loopvarcapture/testdata/src/p/p.go index 09d90a1ad841..b1e7f5ae58a8 100644 --- a/pkg/testutils/lint/passes/loopvarcapture/testdata/src/p/p.go +++ b/pkg/testutils/lint/passes/loopvarcapture/testdata/src/p/p.go @@ -11,6 +11,7 @@ package p import ( + "context" "fmt" "math/rand" "net" @@ -430,3 +431,45 @@ func RespectsNolintComments() { wg.Wait() } } + +// regression test for https://github.com/cockroachdb/cockroach/issues/102678 +func registerKVScalability() { + runScalability := func(ctx context.Context, percent int) { + nodes := 3 + + const maxPerNodeConcurrency = 64 + for i := nodes; i <= nodes*maxPerNodeConcurrency; i += nodes { + fmt.Println("running workload") + + m := NewMonitor(ctx) + m.Go(func(ctx context.Context) error { + cmd := fmt.Sprintf("./workload run kv --init --read-percent=%d "+ + "--splits=1000 --duration=1m "+fmt.Sprintf("--concurrency=%d", i), // want `loop variable 'i' captured by reference` + percent, nodes) + + return fmt.Errorf("failed to run workload: %s", cmd) + }) + } + } + runScalability(nil, 0) +} + +type Monitor interface { + Go(fn func(context.Context) error) +} + +type monitorImpl struct { + ctx context.Context +} + +func (m monitorImpl) Go(fn func(context.Context) error) { + panic("implement me") +} + +func NewMonitor(ctx context.Context) Monitor { + return newMonitor(ctx) +} + +func newMonitor(ctx context.Context) *monitorImpl { + return &monitorImpl{ctx: ctx} +}