Merge #156464

156464: kvserver: don't proceed with unsafe replication changes after 10s r=tbg a=tbg

See #152604 (comment).

`within10s` is called from within the change replicas txn as a last layer
of defense against replication changes that would lose quorum. While looking
into cases in which precisely this happened, I wondered why this last line
of defense couldn't block these ill-advised changes.

It turns out that within10s had a bug: it would return without an error.
In effect, it wasn't doing anything except delay the problematic change.

The bug is fixed here. In follow-up #156463 (not for backporting),
we completely remove this bespoke helper and "just" use `retry`.

This bug is mine - introduced almost five years ago in #57564. Consequently,
it's present in "all" versions of CockroachDB.

Closes #156466.

Epic: none
Release note(bug fix): A mechanism that blocks replication changes that would
result in a loss of quorum was ineffective. It now works as intended.


Co-authored-by: Tobias Grieger <[email protected]>

Author: craig[bot]

pkg/kv/kvserver/replica_command.go

@@ -2402,6 +2402,9 @@ func within10s(ctx context.Context, fn func() error) error {
 			return nil
 		}
 	}
+	if err != nil {
+		return err
+	}
 	return ctx.Err()
 }