cockroachdb / cockroach

CockroachDB — the cloud native, distributed SQL database designed for high availability, effortless scale, and control over data placement.
https://www.cockroachlabs.com
Other
30.11k stars 3.81k forks source link

kvclient/rangefeed: double counting metrics during rangefeed restarts #129486

Open wenyihu6 opened 2 months ago

wenyihu6 commented 2 months ago

Describe the problem

  1. I think we are double counting metrics here https://github.com/cockroachdb/cockroach/blob/e5b30e4a2344a4b0ab1b2cf6a40b134192aeed3d/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go#L530 and again in https://github.com/cockroachdb/cockroach/blob/e5b30e4a2344a4b0ab1b2cf6a40b134192aeed3d/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go#L544.

This double counting happens every time when a rangefeed restart happens which could be often.

  1. Another thing is that - restartActiveRangeFeeds https://github.com/cockroachdb/cockroach/blob/e5b30e4a2344a4b0ab1b2cf6a40b134192aeed3d/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go#L515 has an early return after receiving an error, so some toRestart https://github.com/cockroachdb/cockroach/blob/e5b30e4a2344a4b0ab1b2cf6a40b134192aeed3d/pkg/kv/kvclient/kvcoord/dist_sender_mux_rangefeed.go#L417 resources are not properly released if it returns an error early. But everything should be torn down after startNodeMuxRangeFeed returns an error iiuc. Not sure if resources are properly releasing after shutdown. Still reading more.

I attempted to fix these issues in this draft PR, but the flow control here is complicated, and I haven’t fully read the resource management code in kvclient, so I’m not confident in my understanding. The effort required to gain the necessary confidence seems too high at this point to push the PR over the finish line, so I’m filing an issue instead.

Jira issue: CRDB-41566

Epic CRDB-39959

blathers-crl[bot] commented 2 months ago

Hi @wenyihu6, please add branch-* labels to identify which branch(es) this C-bug affects.

:owl: Hoot! I am a Blathers, a bot for CockroachDB. My owner is dev-inf.