kv,sql: replica command errors bubble up as-is to SQL clients

[knz]

Found in #49513, a KV error went through all the layers and popped up in the client unadulterated:

pq: unable to select removal target from [(n1,s1):?]; current replicas [(n2,s2):1 (n7,s7):2 (n1,s1):3 (n8,s8):4]: could not select an appropriate replica to be removed

The present issue is not about investigating the cause of the error and instead focuses on the communication of this error throughout CockroachDB.

There are a couple of problems with this situation:

• it does not carry a pg error code
• it does not provide SQL-level context (which type of operation was being performed)
• it does not suggest whether the operation is retryable
• it probably is overly confusing - there's nothing about the replica details that's remotely useful to the user of a SQL client.

The way forward would be to address this in SQL by:

• logging the details of the error via %+v in the log file
• replacing the details by a SQL-level summary of the situation (via errors.Handled())

cc @andreimatei if you have suggestions as to how/where to do this? Perhaps the error could be summarized by txncoordsender before it comes into SQL?

cc @asubiotto @jordanlewis for triage.

Jira issue: CRDB-4177

[andreimatei]

I tried tracking through the allocator code to figure out what the error really means and how expected it is, but I got lost quickly. I think the error is generated by an AdminRelocateRange request (which is an unusual one), so I guess if we'd want a better error experience, we'd need to introduce a new ErrorDetail specific to this request type.
Or maybe the answer here is some sort of retry in the store.AdminRelocateRange(). But again I don't really know what the error means.

[knz]

Can you help us determine who would know?

[andreimatei]

I've got nothing. We can see if @tbg or @darinpp know anything, but probably any of us are just as good.
FWIW this test already ignores a bunch of errors coming from EXPERIMENTAL_RELOCATE:

cockroach/pkg/cmd/roachtest/bank.go

Line 330 in f80ec9c

func isExpectedRelocateError(err error) bool {

I think we should keep around #49513 for deflaking the test, but otherwise close this issue frankly. Focusing on the error ergonomics for experimental_relocate in general, and this one error in particular, doesn't seem too important to me.

[tbg]

Here's another instance (internal slack)