kvserver: remove unused method arguments

[pav-kv]

Release note: None

[cockroach-teamcity]

This change is 

[pav-kv]

@erikgrinaker Good to know that, I'll add a comment.

[erikgrinaker]

That said, I believe the recent changes to Go function call conventions in 1.17 may have broken this. I forget what the current state is, maybe you remember @tbg? We should really be using pprof labels instead, if possible.

[erikgrinaker]

Yeah, there's something about it here:

https://github.com/cockroachdb/cockroach/blob/c7396eea02cb07ff4cae954c5051e7f63759abf7/pkg/kv/kvserver/replica_send.go#L117-L136

FWIW, we often call these _forStacks in function definitions to mark them as such:

https://github.com/cockroachdb/cockroach/blob/566dff233d94aa18e2f9e53cdf888adf9ee22d53/pkg/kv/kvserver/replica_rangefeed.go#L142-L144

[tbg]

The unused nodeID parameter was introduced in https://github.com/cockroachdb/cockroach/pull/8529/files#diff-1fa05e654163c64041e809fec538526cedc6723e47797f1f4e5df8993a1e60a9R247 and I don't think the intention was to be able to pull the nodeID from a stack trace. I also don't think the connection class parameter has this purpose, or we wouldn't have tacked it on at the end.

As Erik pointed out, the "correct" way to do this now is pprof annotations, which were added to the goroutine profile output recently!

If you don't mind, @pavelkalinnikov, add that here. Basically, we want to wrap this call

https://github.com/cockroachdb/cockroach/blob/0b734081e97448944b7dc26ee9a3097cf9f96d63/pkg/kv/kvserver/raft_transport.go#L713-L715

in a pprof.Do:

		pprof.Do(ctx, pprof.Labels("remote_node_id", toNodeID.String()), func(ctx context.Context) {
			if err := t.processQueue(toNodeID, ch, stats, stream, class); err != nil {
				log.Warningf(ctx, "while processing outgoing Raft queue to node %d: %s:", toNodeID, err)
			}
		})

You can then ./dev build, run a (say) three-node cluster, and navigate to /debug/pprof/goroutine?debug=1 and look for a corresponding labels: line like this (but referencing your label).

---

For Replica.Send (i.e. fix up sendWithoutRangeID to use the same pattern) it's unfortunately harder due to the need for allocations while also wanting to reuse incoming label sets if there is one, which the API also makes difficult to discover in a low-alloc way (though not impossible). Filed https://github.com/cockroachdb/cockroach/issues/85948 for this.

[pav-kv]

@tbg @erikgrinaker Done, PTAL.

[pav-kv]

If you don't mind, @pavelkalinnikov, add that here. Basically, we want to wrap this call

@tbg I wrapped it a bit higher up the stack, in the place where the worker is launched.

[pav-kv]

bors r=tbg,erikgrinaker

[craig[bot]]

Build succeeded:

• Bazel Essential CI (Cockroach)