kv: properly handle ambiguous errors that occur alongside commits

[AlexTalks]

As observed in the wild (#67765), in autonomous simulation testing (internal link), and in our own reproductions (#103123, #103756), we have several bugs relating to the way we handle ambiguous errors in KV batches that commit, potentially manifesting as transaction unexpectedly committed errors (which will cause a node crash), or as transactions that are incorrectly reported to the client as failed when they may actually be end up being explicitly committed.

These ambiguous errors - i.e., a network error on one of a transaction's BatchRequest RPCs that cannot be determined to if it has succeeded or failed - are inherent to any distributed system, but need to be handled properly in order to avoid issues with our implementation of the parallel commit protocol. When one of these errors occurs in the DistSender on a batch that contains an EndTxn request, we currently attempt to elide the error like so:

cockroach/pkg/kv/kvclient/kvcoord/dist_sender.go

Lines 2235 to 2237 in 775b005

	if withCommit && !grpcutil.RequestDidNotStart(err) {
	ambiguousError = err
	}

While we attempt to reissue (potentially: idempotently replay) any failed BatchRequests, sending them to subsequent replicas in the replica set in an attempt to sidestep the ambiguous error, this can run into problems:

• If the transaction actually did succeed in all of its writes, the transaction would be implicitly committed (and in transaction state STAGING) despite the ambiguous error.
• In this state, some other operation encountering the transaction record (e.g. contending on keys) may explicitly commit it via a RecoverTxn request.
• Since the transaction coordinator is unaware of this recovery, it may result in a duplicate EndTxn request at a higher timestamp (transaction unexpectedly committed) or even fail on the reissued request, reporting a transaction failure to the client for a transaction that was ultimately committed.

This should be fixed for both correctness in the near term, and improvement in the longer term. A discussed approach is as follows:

Near Term:
As a quicker fix, we should avoid attempting to sidestep the ambiguous errors on batches with commits in the DistSender and instead propagate them as AmbiguousResultErrors.

• This avoids the potential issues caused by reissuing the requests after the ambiguous error, and indicates to the client that the transaction's commit state needs to be independently validated.
• This approach should be validated that it does not cause a marked increase in unnecessary/avoidable AmbiguousResultErrors, and should be tested using the data-driven test reproductions linked above.

Medium Term:
We should be able to avoid raising all RPC failures as AmbiguousResultErrors, and minimize the cases to only those necessary by "remembering" the earlier ambiguity in retrying our failed writes and rejecting them as non-idempotent if they change the timestamp of a transaction that may already be implicitly committed.

• This avoids potentially reporting a transaction as incorrectly "failed" or "double committing", instead returning an ambiguous error only in the necessary cases, indicating to the client that the transaction's commit state needs to be idependently validated.

Longer Term:
See #108342.

Jira issue: CRDB-28217

[daniel-crlabs]

Affects this customer: https://cockroachdb.zendesk.com/agent/tickets/19344

[daniel-crlabs]

Affects this customer: https://cockroachdb.zendesk.com/agent/tickets/19344