cockroachdb / cockroachdb/pebble
wal: failover monitor updates dir state then discards switchToNewDir limit error
- 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