changefeedccl: transient schemafeed errors are not retried

[stevendanna]

Describe the problem

In #63282 we saw that a clock drift issue led to one node being terminated. This then resulted in a changefeed failure because the node termination led to a failure to fetch a table descriptor during schema polling.

It appears that all errors from the schemafeed are treated as fatal, even though they might be caused by transient communication issues.

We may want identify which of the many error returns in fetchDescriptorVersions may be retriable. It seems likely that at least this ought to be:

	res, pErr := kv.SendWrappedWith(ctx, db.NonTransactionalSender(), header, req)
	if log.V(2) {
		log.Infof(ctx, `fetched table descs (%s,%s] took %s`, startTS, endTS, timeutil.Since(start))
	}
	if pErr != nil {
		err := pErr.GoError()
		return nil, errors.Wrapf(err, `fetching changes for %s`, span)
	}

Note thatpErr.GoError() will return an UnhandledRetriableError in some cases, but the rest of the changefeed code doesn't look for that particular type. And, it is isn't clear to me without further verification that that will cover all of the transient errors the function could return.

Jira issue: CRDB-6524

[miretskiy]

I honestly think that for sink-based changefeeds, we should consider switching error handling model from "errors are fatal, unless marked as retryable" to "errors are retryable, unless marked fatal". We should also augment retries with addressing #62556 (do not retry forever without making progress).

[stevendanna]

I honestly think that for sink-based changefeeds, we should consider switching error handling model from "errors are fatal, unless marked as retryable" to "errors are retryable, unless marked fatal".

I could get behind this, especially if we do a good job on #62556

[knz]

Retryable errors are already reliably marked as such, see how the pgerror package detects them.

[ajwerner]

My feeling for defaulting to retrying for jobs on errors is that #44594 should be a prerequisite. Having a job spin on failure seems very bad.

[stevendanna]

Retryable errors are already reliably marked as such, see how the pgerror package detects them.

Assuming it correctly marks all retriable errors, there is still work here on the consumer side to make sure the changefeed machinery looks for and responds to the correct retriable types.

My feeling for defaulting to retrying for jobs on errors is that #44594 should be a prerequisite. Having a job spin on failure seems very bad.

Seems reasonable.