cockroachdb / cockroachdb/cockroach

raft: eliminate log scan on campaigns

Open
#131,559 1 comment 1 reaction 0 assignees View on GitHub
A-kv-replication C-enhancement C-performance T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

The [config change](https://github.com/etcd-io/etcd/issues/7625#issuecomment-489232411) implementation in `raft` package requires a candidate to check whether the raft log [has](https://github.com/cockroachdb/cockroach/blob/5400cb9a70e63bfe1aa2849a566c195ad63130d1/pkg/raft/raft.go#L1157-L1160) unapplied config changes. This is implemented as a [log scan](https://github.com/cockroachdb/cockroach/blob/5400cb9a70e63bfe1aa2849a566c195ad63130d1/pkg/raft/raft.go#L1183-L1209), which potentially reads from log storage.

We would like to eliminate cases of uncontrolled IO from inside `raft`, which this scan classifies as.

One way to eliminate this scan:
- Track the last **committed** config change log index (`confCommit`), similarly to how we track the [last pending](https://github.com/cockroachdb/cockroach/blob/5400cb9a70e63bfe1aa2849a566c195ad63130d1/pkg/raft/raft.go#L387-L393) config change index.
- Store this index in hard state alongside the [committed index](https://github.com/cockroachdb/cockroach/blob/5400cb9a70e63bfe1aa2849a566c195ad63130d1/pkg/raft/raftpb/raft.proto#L133), so that it survives restarts.
- Then the "has unapplied config changes" check can be implemented as: `applied < confCommit`.

This change probably requires a migration: upon startup, scan all raft logs (alternatively, limit the scan to unapplied entries only), and initialize this variable in storage. This change would be best bundled with any other changes that need a full-scan raft log migration.

Jira issue: CRDB-42602

Contributor guide

Open the contributing guide

Research direction

Start with the raft implementation in pkg/raft/raft.go, especially the pending configuration tracking and log-scan check, then inspect the committed-index definition in pkg/raft/raftpb/raft.proto. Determine how startup storage migration would initialize the committed configuration index. Done means campaigns no longer scan raft log storage for unapplied configuration changes and the index survives restarts.

Written by the indexing model from the issue text.

Assessment

Tech stack
go
Domain
databases, distributed-systems
Issue type
Refactor
Difficulty
4/5
Estimated time
3-5 days
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
28/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.