cockroachdb / cockroachdb/pebble

wal: failover monitor updates dir state then discards switchToNewDir limit error

Open
#6,173 0 comments 0 reactions 0 assignees View on GitHub
Dominant language
Go
Stars
6k
Forks
584
Avg merge
16h 35m
Merged PRs (30d)
5

Description

## Summary

When WAL failover hits the per-WAL physical-log cap (`maxPhysicalLogs = 10`), `failoverWriter.switchToNewDir` returns `"exceeded switching limit"`. The failover monitor updates its directory state first, then discards that error. After the cap, the monitor's view of the active directory oscillates on every sample tick while the writer stays on the last physical log.

## What happens

In `failoverMonitor.monitorLoop`, a decided switch does all of the following before the writer call:

1. Flip the local `dirIndex` and enable or disable the primary prober
2. Under `m.mu`, update `dirIndex`, `dirSwitchCount`, duration accounting, and `lastFailBackTime`
3. Call `switchToNewDir` and ignore the result:

```go
if m.mu.writer != nil {
_ = m.mu.writer.switchToNewDir(dir)
}
```

`switchToNewDir` is documented as returning a non-nil error only when the switching limit is exceeded. That is a synchronous failure: no new writer is created.

After the 10th successful switch, every later sample that still looks unhealthy:

- flips `dirIndex` again
- increments `dirSwitchCount`
- rewrites `lastFailBackTime` on fail-back ticks
- fails `switchToNewDir` again

Default `UnhealthySamplingInterval` is 100ms, so this can happen 10 times per second until the virtual WAL rotates.

## Why it matters

This is not a record-loss path. The writer keeps using the last physical log. The damage is monitor state:

- Primary vs secondary duration metrics become wrong
- `dirSwitchCount` keeps climbing after switching is no longer possible
- `lastFailBackTime` and probe enable/disable no longer match the writer
- Later heuristics (error-count damping, fail-back) run against a dir the writer is not using

A flapping or dual-error disk can hit 10 switches quickly (an error triggers an immediate switch). That is a reachable WAL-failover configuration in CockroachDB.

## Suggested fix

Call `switchToNewDir` before committing monitor state, or roll back `dirIndex` / probe state / `lastFailBackTime` / `dirSwitchCount` when it returns `"exceeded switching limit"`. After the cap, stop trying to switch this writer.

A datadriven case in `wal/testdata` that forces 10 switches and then another unhealthy sample would lock the expected monitor state.

## Origin

| Lines | Commit | Date | Author |
|-------|--------|------|--------|
| 473-496 | [`2154c3729`](https://github.com/cockroachdb/pebble/commit/2154c3729) | 2024-01-13 | sumeerbhola |
| `_ =` discard | [`d8b52a2b3`](https://github.com/cockroachdb/pebble/commit/d8b52a2b3) | 2025-04-08 | Radu Berinde (errcheck sweep) |

`maxPhysicalLogs` is defined in [`wal/failover_writer.go`](https://github.com/cockroachdb/pebble/blob/master/wal/failover_writer.go) (`const maxPhysicalLogs = 10`).

Jira issue: PEBBLE-1471

Contributor guide

No contributing guide indexed for this repository

Research direction

Start in wal/failover_monitor.monitorLoop and wal/failover_writer.go's switchToNewDir to trace when the switching-limit error is returned and how monitor state is committed. Add the suggested wal/testdata datadriven case for 10 switches followed by an unhealthy sample; done means the writer remains on the last physical log and monitor state no longer oscillates or counts failed switches.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases
Issue type
Bug
Difficulty
3/5
Estimated time
1-2 days
Activity status
Active
Clarity
Clearly specified
Newbie friendliness
68/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.