kv: avoid immediately launching goroutine for txn heartbeat loop

[nvanbenschoten]

We currently launch a goroutine immediately for any transaction that doesn't send an EndTransaction in the same batch as its first write. This is a little crazy because very few transactions will ever actually run long enough to send even their first heartbeat after 1s. As an non-invasive way to improve this, we could introduce a new Stopper.RunDelayedAsyncTask method which waits for some specified period of time (in a single goroutine) before launching the async task. While waiting, the goroutine coordinating this could be monitoring context cancellation to remove queued async tasks before they fire off their own goroutine.

Using this approach, we could trigger a delayed async task in the heartbeater when we see the first write and delay the goroutine for an entire heartbeat interval, only paying the cost of the goroutine for transactions that will send at least one heartbeat.

An alternative to this is to create a single goroutine that handles transmitting all transaction heartbeats, but as has been discussed before, this will require protocol changes and migrations. If we're planning on taking something that large on, we should first consider alternative means of tracking transaction liveness, as we discuss here:

cockroach/pkg/kv/txn_interceptor_heartbeater.go

Lines 50 to 59 in b662cb6

	// NOTE: there are other mechanisms by which concurrent actors could determine
	// the liveness of transactions. One proposal is to have concurrent actors
	// communicate directly with transaction coordinators themselves. This would
	// avoid the need for transaction heartbeats and the PENDING transaction state
	// entirely. Another proposal is to detect abandoned transactions and failed
	// coordinators at an entirely different level - by maintaining a node health
	// plane. This would function under the idea that if the node a transaction's
	// coordinator is running on is alive then that transaction is still in-progress
	// unless it specifies otherwise. These are both approaches we could consider in
	// the future.

Interestingly, there's already another case that would benefit from this in the txnPipeliner. We've always wanted to add a background job that periodically "proves" in-flight writes. Using a Stopper.RunDelayedAsyncTask method would allow us to add in this job without seeing a performance hit for short-running transactions. This would help us stay beneath the max inflight-write bytes limit we'll need to add for #32522.

cc. @ajwerner. I expect that the internal mechanism here will look very similar to what you added in requestbatcher.

Jira issue: CRDB-4614

[ajwerner]

Does the txnCoordSender have a defined lifecycle? The reason I ask is about when cleanup happens for the DelayedAsyncTasks. It is true that we could do it lazily based on context cancellation but it might be nice to have a mechanism to remove tasks from the delay queue eagerly.

[ajwerner]

Ignore the above comment, I found what I was looking for

[nvanbenschoten]

Yeah, I was thinking we would remove txnEnd and instead replace it with a context cancellation function tied to the existing hbCtx.

[github-actions]

We have marked this issue as stale because it has been inactive for
18 months. If this issue is still relevant, removing the stale label
or adding a comment will keep it active. Otherwise, we'll close it in
5 days to keep the issue queue tidy. Thank you for your contribution
to CockroachDB!

[knz]

still relevant

[ajwerner]

I'm taking this off my list.

[nvanbenschoten]

This was addressed by #67215.