kvserver: fix replicate queue metrics tracking bug #86844
Conversation
|
This change is |
| @@ -481,6 +481,12 @@ func (metrics *ReplicateQueueMetrics) trackResultByAllocatorAction( | |||
| } else { | |||
| metrics.ReplaceDeadReplicaErrorCount.Inc(1) | |||
| } | |||
| case allocatorimpl.AllocatorRemoveDeadVoter, allocatorimpl.AllocatorRemoveDeadNonVoter: | |||
There was a problem hiding this comment.
Should this case be folded into the one right above?
pkg/kv/kvserver/replicate_queue.go
Outdated
| @@ -494,7 +500,7 @@ func (metrics *ReplicateQueueMetrics) trackResultByAllocatorAction( | |||
| metrics.RemoveDecommissioningReplicaErrorCount.Inc(1) | |||
| } | |||
| default: | |||
| panic(fmt.Sprintf("unsupported AllocatorAction: %v", action)) | |||
| // Unsupported AllocatorAction. | |||
There was a problem hiding this comment.
Worth log.Errorf-ing, or fatal-ing under buildutil.CrdbTestBuild.
|
The reason this wasn't caught in tests was because |
There was a problem hiding this comment.
Reviewable status:
complete! 0 of 0 LGTMs obtained (waiting on @AlexTalks and @irfansharif)
pkg/kv/kvserver/replicate_queue.go line 484 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
Should this case be folded into the one right above?
No, we have separate metrics for ReplaceDead and RemoveDead.
pkg/kv/kvserver/replicate_queue.go line 503 at r1 (raw file):
Previously, irfansharif (irfan sharif) wrote…
Worth log.Errorf-ing, or fatal-ing under
buildutil.CrdbTestBuild.
Will do.
AlexTalks
force-pushed
the
fix_rq_metrics
branch
from
053abc0 to
28ede05
Compare
There was a problem hiding this comment.
Dismissed @irfansharif from a discussion.
Reviewable status:
pkg/kv/kvserver/replicate_queue.go line 503 at r1 (raw file):
Previously, AlexTalks (Alex Sarkesian) wrote…
Will do.
Done.
|
bors r+ |
|
Build failed (retrying...): |
|
Build failed (retrying...): |
|
bors cancel |
|
Canceled. |
In cockroachdb#85844, tracking replicate queue success and error metrics by allocator action was introduced, however this change inadvertently included a bug due to missing the `RemoveDead(Non)Voter` allocator action case, in combination with a panic on unexpected actions. This change fixes the missing case as well as the panic. Release justification: Bug fix Release note: None
AlexTalks
force-pushed
the
fix_rq_metrics
branch
from
28ede05 to
cbbdbcb
Compare
|
bors r+ |
|
Build succeeded: |
In #85844, tracking replicate queue success and error metrics by
allocator action was introduced, however this change inadvertently
included a bug due to missing the
RemoveDead(Non)Voterallocatoraction case, in combination with a panic on unexpected actions. This
change fixes the missing case as well as the panic.
Fixes #86544, fixes #86374, fixes #86107.
Release justification: Bug fix
Release note: None