kvclient/rangefeed: clients must handle internal errors

[erikgrinaker]

Rangefeed clients based on kvclient/rangefeed.Factory must explicitly set an OnInternalError handler to be notified of internal errors. Otherwise, when one of these errors are encountered the rangefeed will silently stop emitting events. The current error conditions are:

• roachpb.BatchTimestampBeforeGCError: the rangefeed sent a batch request below the GC threshold.
• roachpb.MVCCHistoryMutationError: MVCC history was mutated by e.g. AddSSTable, ClearRange, or RevertRange.

Callers should add a callback using WithOnInternalError and handle these errors appropriately (as well as any future error conditions that may be added). Relevant callers include:

• SQL descriptor watcher
• SQL zone watcher
• SQL lease watcher
• SQL stats cache watcher
• Settings watcher
• Span config subscriber

CC @cockroachdb/sql-schema @irfansharif

Jira issue: CRDB-12744

[ajwerner]

This is legit. I have a PR to address this of for the settings and I think also the span configs. I'll revisit this later, as it's now easy enough to do with the pattern in #74612.

[ajwerner]

Turns out I missed the OnUnrecoverableError callback in #74612. Hat tip to @RaduBerinde for spotting it.

[ajwerner]

I'll note that at this point this is something, of a paranoia issue. Yes, we should do it, but it's not that pressing. There are two cases where these rangefeeds will fail in an unrecoverable way: the underlying span gets an AddSSTable or the GC TTL passes while the span is being watched. Those spans never get written to with AddSSTable, so that’s not really an issue. The GC TTL Is currently 25h, so these system tables would need to be unavailable for at least that long (though in practice, much longer) for this error to be hit.

Anyway, I do plan to do something here, just want to clarify the urgency.

[erikgrinaker]

I agree, this seems very unlikely -- I'm just a bit paranoid that some of these MVCC bulk ops changes can have unintended consequences, and silent failures are the worst kind of failure. Even just logging an error would be fine for a start.