cockroachdb / cockroachdb/cockroach

kvserver: minLeaseProposedTS proscription guard bypassed with separated engines

Open
#173,625 1 comment 0 reactions 1 assignee Claimed by @pav-kv View on GitHub
A-kv branch-master C-bug O-agent T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

With separated engines, the `minLeaseProposedTS` proscription guard that prevents reuse of stale leases after restart is ineffective. A restarted node can serve reads under a pre-restart lease without acquiring a new one.

**Root cause:**

The state engine runs with `DisableWAL()`. Lease writes (regular raft applies) commit with `sync=false` to the state engine's MemTable. On `Close()`, Pebble does not flush MemTables — it expects WAL replay, but there is no WAL. After restart, the lease data is missing from the state engine.

WAG replay cannot recover the lease because WAG only tracks replica lifecycle events and ingest/excise commands (`replica_app_batch.go:204`), not regular raft applies like lease requests.

Raft replay *can* recover the lease (replaying committed-but-unapplied entries from the log engine), but it runs too late. By the time `processRaft` starts (`store.go:2667`), `initRaftMuLockedReplicaMuLocked` has already checked `r.shMu.state.Lease.Sequence` (`replica_init.go:371`). Since the latest lease was not applied at that point, `minLeaseProposedTS` was not set.

Later, raft replay restores the lease, but the restored pre-restart lease is considered `VALID` instead of `PROSCRIBED`, and the node serves reads under it without acquiring a new one.

**Reproduction:**

`TestLeaseNotUsedAfterRestart` exposes this. Remove the `DisableSeparatedEngines` knob, and run with separated engines forced on:

```
./dev test pkg/kv/kvserver -f=TestLeaseNotUsedAfterRestart --count=10 \
-- --test_env=COCKROACH_INTERNAL_METAMORPHIC_OVERRIDES=separated-engines=true
```

Result: 9/10 failures. Non-separated case passes 30/30. The test is currently skipped for separated engines.

**Code references:**

- State engine `DisableWAL`: [`server/config.go:1006`](https://github.com/cockroachlabs/cockroach/blob/master/pkg/server/config.go#L1006)
- `minLeaseProposedTS` set based on loaded state: [`kvserver/replica_init.go:371-387`](https://github.com/cockroachlabs/cockroach/blob/master/pkg/kv/kvserver/replica_init.go#L371-L387)
- Raft processing starts after replica init: [`kvserver/store.go:2667`](https://github.com/cockroachlabs/cockroach/blob/master/pkg/kv/kvserver/store.go#L2667)
- State batch commits without sync: [`kvserver/kvstorage/storage.go:538-555`](https://github.com/cockroachlabs/cockroach/blob/master/pkg/kv/kvserver/kvstorage/storage.go#L538-L555)

Jira issue: CRDB-66934
Epic: CRDB-66033

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.