kv: don't return ReplicaCorruptionError from commit trigger evaluation due to context cancellation

[nvanbenschoten]

We currently wrap any error from splitTrigger or mergeTrigger evaluation with a ReplicaCorruptionError. This is incorrect in the case of a context cancellation error, which is possible because request evaluation runs before raft, in the client's context.

We should return the errors directly in these cases, without wrapping them in corruption errors. Something like:

diff --git a/pkg/kv/kvserver/batcheval/cmd_end_transaction.go b/pkg/kv/kvserver/batcheval/cmd_end_transaction.go
index e21903fe631..20b5521fcf2 100644
--- a/pkg/kv/kvserver/batcheval/cmd_end_transaction.go
+++ b/pkg/kv/kvserver/batcheval/cmd_end_transaction.go
@@ -779,7 +779,10 @@ func RunCommitTrigger(
                        ctx, rec, batch, *ms, ct.SplitTrigger, txn.WriteTimestamp,
                )
                if err != nil {
-                       return result.Result{}, kvpb.NewReplicaCorruptionError(err)
+                       if !errors.Is(err, ctx.Err()) {
+                               err = kvpb.NewReplicaCorruptionError(err)
+                       }
+                       return result.Result{}, err
                }
                *ms = newMS
                return res, nil
@@ -787,7 +790,10 @@ func RunCommitTrigger(
        if mt := ct.GetMergeTrigger(); mt != nil {
                res, err := mergeTrigger(ctx, rec, batch, ms, mt, txn.WriteTimestamp)
                if err != nil {
-                       return result.Result{}, kvpb.NewReplicaCorruptionError(err)
+                       if !errors.Is(err, ctx.Err()) {
+                               err = kvpb.NewReplicaCorruptionError(err)
+                       }
+                       return result.Result{}, err
                }
                return res, nil
        }

Jira issue: CRDB-33515

[nvanbenschoten]

@dt found another case where we make the same mistake:

cockroach/pkg/kv/kvserver/replica.go

Lines 2320 to 2321 in bab4335

	return kvpb.NewError(kvpb.NewReplicaCorruptionError(
	errors.Wrap(err, "could not read from AbortSpan")))

We should audit all of these calls to NewReplicaCorruptionError. We can also add some assertions in that construct that the error does not have a context timeout/cancellation cause.

[dt]

Should we rename NewReplicaCorruptionError to MaybeWrapReplicaCorruptionError and just make it take a ctx and do the check itself?