cockroachdb / cockroachdb/cockroach

backup: intent resolver batcher timeout retry is unbounded, unlike WriteIntentError retry

Open
#173,354 3 comments 0 reactions 1 assignee Claimed by @dt View on GitHub
A-disaster-recovery C-bug O-agent P-2 T-disaster-recovery
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Describe the problem**

[#168270](https://github.com/cockroachdb/cockroach/pull/168270) (commit `02c2c76`) made `runBackupProcessor` retry an `ExportRequest` when it fails with a `*timeutil.TimeoutError` whose operation name contains `"intent_resolver"` (an intent-resolver-batcher timeout), treating it like a `WriteIntentError`. The new path reuses the existing retry action but sits **outside** the state machine that guarantees the `WriteIntentError` retry terminates. As a result, a persistent intent-resolver-batcher timeout can cause a backup to retry the same span forever instead of failing, turning a fast, loud failure into a silent hang until the job is cancelled.

**Why the `WriteIntentError` retry is bounded**

The `WriteIntentError` retry is bounded in the following manner:

1. The retry loop runs in one of two regimes, selected by the `priority` bool. In the non-priority regime it sets [`header.WaitPolicy = lock.WaitPolicy_Error`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L516), asking KV to return a `WriteIntentError` rather than block. In the priority regime it instead sets [`header.UserPriority = roachpb.MaxUserPriority`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L507) with a blocking wait, so KV pushes/aborts the conflicting txns and forces the read through.
2. A `WriteIntentError` can therefore **only be produced in the non-priority regime**, and the retry condition re-checks that gate: [`isWriteIntentErr && header.WaitPolicy == lock.WaitPolicy_Error`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L575).
3. The loop transitions non-priority → priority permanently once [`timeutil.Since(readTime) > read_with_priority_after`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L491). Each retry advances wall-clock time, so this condition is eventually met.

Once the loop enters the priority regime, `WaitPolicy_Error` is no longer set, KV blocks-and-pushes instead of returning `WriteIntentError`, and the retry branch can no longer fire. Forward progress is guaranteed by the escalation from "politely back off" to "abort whatever is in the way."

**Why the intent-resolver-batcher timeout retry is not bounded**

The new predicate is OR'd into the same condition but is **not gated on `WaitPolicy`**:

```go
lockErr, isWriteIntentErr := pErr.GetDetail().(*kvpb.WriteIntentError)
if (isWriteIntentErr && header.WaitPolicy == lock.WaitPolicy_Error) || isIrBatcherTimeout {
```
([backup_processor.go#L575](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L575))

Two things break the termination argument:

- **It fires in both regimes.** Entering the priority regime does not stop `isIrBatcherTimeout` retries.
- **Priority escalation doesn't clear the failure.** `MaxUserPriority` changes *who wins a lock conflict*; it does nothing to speed up an intent resolver that is timing out because it is slow or overloaded. So the timeout can keep recurring even under `MaxUserPriority`.

There is no cap on [`span.attempts`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L579) (it is only incremented and read for the priority decision), and a backup has no overall wall-clock deadline. So a span whose intent-resolver-batcher timeout does not self-resolve is retried indefinitely. Before #168270 this same condition surfaced quickly as a terminal error — [`"KV storage layer did not respond to BACKUP within timeout"`](https://github.com/cockroachdb/cockroach/blob/02c2c7641aac7d076d87cd7a7a99882baca06675/pkg/backup/backup_processor.go#L601).

The retry reason is also only logged at `log.VEventf(ctx, 1, ...)`, so an operator watching a stalled backup gets no default-level signal that it is spinning on intent-resolver-batcher timeouts.

Jira issue: CRDB-66670

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.