kv: eliminate heap allocations throughout Raft messaging and processing logic#57007
Conversation
We were already computing this duration one level above, so there's no need to compute it again.
Addresses a TODO. Avoids allocations for small numbers of messages. This is common for inactive ranges that are still heartbeating. It should help with cases like cockroachdb#56860.
This was responsible for an allocation per processReady and processRequestQueue call. This means that it would cause two heap allocations per Range on each replica during a Raft heartbeat. As a result, it was the largest source of allocations by far in tests like cockroachdb#56860 with lots of idle Ranges heartbeating frequently. This change should reduce the cost of handling these heartbeats, reducing the impact the heartbeats have on other active Ranges.
|
This change is |
aayushshah15
left a comment
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @aayushshah15 and @nvanbenschoten)
pkg/kv/kvserver/store_raft.go, line 282 at r3 (raw file):
ctx context.Context, r *Replica, ) (pErr *roachpb.Error) { ctx = r.AnnotateCtx(ctx)
Why aren't we using raftSchedulerCtx here?
nvb
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/store_raft.go, line 282 at r3 (raw file):
Previously, aayushshah15 (Aayush Shah) wrote…
Why aren't we using
raftSchedulerCtxhere?
processRaftSnapshotRequest is not called from the Raft scheduler, it's called from Store.receiveSnapshot with the RPC's stream's context.
aayushshah15
left a comment
There was a problem hiding this comment.
Reviewable status:
pkg/kv/kvserver/store_raft.go, line 282 at r3 (raw file):
Previously, nvanbenschoten (Nathan VanBenschoten) wrote…
processRaftSnapshotRequestis not called from the Raft scheduler, it's called fromStore.receiveSnapshotwith the RPC's stream's context.
👍
|
TFTR! bors r+ |
|
Build succeeded: |
This PR contains a few improvements within the Raft messaging and processing code to avoid common heap allocations that stuck out during tests in #56860. This will improve the general efficiency of the Raft pipeline and will specifically reduce the cost of handling large coalesced heartbeats. This will reduce the impact that these heartbeats have on the scheduler latency of other active ranges.