sql,pgerror: improve IsSQLRetryableError and IsDistSQLRetryable

[ajwerner]

Describe the problem

The string error matching is brittle. The goals are good. Retrying when an error might be permanent no longer hurts in the way it once did: jobs use exponential backoff and record a log of failures. The user can cancel a job if the system makes the wrong decision systematically.

cockroach/pkg/sql/pgwire/pgerror/errors.go

Lines 164 to 166 in da4f6a4

	matched, merr := regexp.MatchString(
	"(no inbound stream connection|connection reset by peer|connection refused|failed to send RPC|rpc error: code = Unavailable|EOF|result is ambiguous)",
	errString)

Jira issue: CRDB-16692

[ajwerner]

https://cockroachlabs.slack.com/archives/C0HM2DZ34/p1655150171201549 contains some internal discussion on the topic.

[knz]

@dt's summary:

currently the jobs system has a retry loop with built-in backoff, and many jobs themselves have retry loops either around their entire work function or specific phases. The generic jobs system one and its exponential backoff is not always appropriate for all jobs, nor is its generic “should retry” logic easy to maintain to the satisfaction of all different jobs. On the other hand, if a job’s code crashes a node, the job’s retry/backoff/etc code doesn't have the option to do its own, more specific thing, and we need this system level backoff around even invoking the jobs code at all to avoid crashing all nodes in quick succession.

One proposed answer is to simplify/dumb down the system-builtin logic to have no retry on error, and thus no need to try to classify different errors from different jobs in one place. The system would instead just rate limit how often it is willing to call the job code between getting some sort of return value from it: record timestamp, invoke job. If you notice job needs to be invoked again (because the node that invoked it crashed), wait some minimum time (60s?). In any other situation, it is up to the job itself to elect to retry certain errors, how long to wait between those tries and when to emit a terminal success or failure. As one special case, emitting an error with a particular marker could be non-terminal and just surrender the execution lease which would clear the invoke backoff and allow another node to invoke immediately

[ajwerner]

I think that summary is somewhat orthogonal to this issue; it is something of an aside related to retry responsibility of jobs. The function here is not generic for the whole job system, it's just used by a couple of jobs to decide whether whether to move to the failed state or not.