streamhelper/advancer: Fix owner transfer metric #42422
Conversation
|
[REVIEW NOTIFICATION] This pull request has not been approved. To complete the pull request process, please ask the reviewers in the list to review by filling The full list of commands accepted by this bot can be found here. Reviewer can indicate their review by submitting an approval review. |
ti-chi-bot
added
release-note
Signed-off-by: hillium <[email protected]>
YuJuncen
force-pushed
the
fix-owner-transfer-metric
branch
from
b318fd2
to
474dc77
Compare
ti-chi-bot
added
size/XS
Signed-off-by: hillium <[email protected]>
| @@ -48,6 +48,7 @@ func (c *CheckpointAdvancer) Name() string { | |||
|
|
|||
| func (c *CheckpointAdvancer) onStop() { | |||
| metrics.AdvancerOwner.Set(0.0) | |||
| metrics.LastCheckpoint.Reset() | |||
There was a problem hiding this comment.
How about other metrics? should also reset them?
There was a problem hiding this comment.
I think perhaps we don't need to reset them before some problems with them evaluated. 🤔
There was a problem hiding this comment.
can we put a comment here saying other metrics potentially need to be audit as well? And maybe a comment describing why do we need to reset checkpoint for this issue? I don't have context on this so just looking at the code I'm very clear what needs to be done. Thanks!
|
/test unit-test
…On 3/21/23, Ti Chi Robot ***@***.***> wrote:
@YuJuncen: The following test **failed**, say `/retest` to rerun all failed
tests or `/retest-required` to rerun all mandatory failed tests:
Test name | Commit | Details | Required | Rerun command
--- | --- | --- | --- | ---
idc-jenkins-ci-tidb/unit-test | 4ad66ba |
[link](https://do.pingcap.net/jenkins/job/pingcap/job/tidb/job/ghpr_unit_test/20094/display/redirect)
| true | `/test unit-test`
[Full PR test
history](https://prow.tidb.net/pr-history?org=pingcap&repo=tidb&pr=42422).
[Your PR
dashboard](https://prow.tidb.net/pr?query=is%3Apr%20state%3Aopen%20author%3AYuJuncen).
<details>
Instructions for interacting with me using PR comments are available
[here](https://git.k8s.io/community/contributors/guide/pull-requests.md).
If you have questions or suggestions related to my behavior, please file an
issue against the
[kubernetes/test-infra](https://github.com/kubernetes/test-infra/issues/new?title=Prow%20issue:)
repository. I understand the commands that are listed
[here](https://go.k8s.io/bot-commands).
</details>
<!-- test report -->
--
Reply to this email directly or view it on GitHub:
#42422 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
|
/test unit-test |
|
@YuJuncen: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
ti-chi-bot
bot
added
approved
needs-1-more-lgtm
|
[APPROVALNOTIFIER] This PR is APPROVED This pull-request has been approved by: 3pointer, Leavrth The full list of commands accepted by this bot can be found here. The pull request process is described here
Needs approval from an approver in each of these files:
Approvers can indicate their approval by writing |
ti-chi-bot
bot
added
lgtm
and removed
needs-1-more-lgtm
|
@YuJuncen: The following tests failed, say
Full PR test history. Your PR dashboard. Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. I understand the commands that are listed here. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #42422 +/- ##
================================================
+ Coverage 73.6423% 80.0476% +6.4052%
================================================
Files 1167 2858 +1691
Lines 365844 834447 +468603
================================================
+ Hits 269416 667955 +398539
- Misses 79076 140293 +61217
- Partials 17352 26199 +8847 |
|
/retest |
|
@BornChanger: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
|
Thanks for the fix! |
|
/retest |
|
@YuJuncen: Cannot trigger testing until a trusted user reviews the PR and leaves an In response to this:
Instructions for interacting with me using PR comments are available here. If you have questions or suggestions related to my behavior, please file an issue against the kubernetes-sigs/prow repository. |
What problem does this PR solve?
Issue Number: close #42419
Problem Summary:
When the owner get evicted but not restarted, its checkpoint metric may consist.
What is changed and how it works?
Check List
Tests
(I think this could be covered by the existing tests in AdvancerDaemon.)
(TBD)
Release note
Please refer to Release Notes Language Style Guide to write a quality release note.