cockroachdb / cockroachdb/cockroach

raftentry: Cache should hold `raftlog.Entry`, not `raftpb.Entry`

Open
#99,778 1 comment 0 reactions 0 assignees View on GitHub
C-enhancement T-kv
Dominant language
Go
Stars
32.5k
Forks
4.1k
PR merge metrics
PR metrics pending

Description

**Is your feature request related to a problem? Please describe.**

a `raftpb.Entry` needs to be unmarshaled, which is expensive. It might make sense to hold the already-unmarshaled `raftlog.Entry`[^1] instead.

[^1]: https://github.com/cockroachdb/cockroach/blob/4df47f54f042355a7b2231a9efc15aa1857d81d5/pkg/kv/kvserver/raftlog/entry.go#L73-L84

**Describe the solution you'd like**

See above.

**Describe alternatives you've considered**

`raftlog.Entry` has a larger memory footprint, so there is a trade-off.

**Additional context**

Our currently raft entry cache sizing seems extremely stingy[^2] and we should revisit it if we decide to address this issue (and even if we don't!)

[^2]: https://github.com/cockroachdb/cockroach/issues/98666

Jira issue: CRDB-26072

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.