Skip to content

metric, tikv: record duration for each backoff type#11710

Merged
sre-bot merged 3 commits into
pingcap:masterfrom
lysu:dev/record-backoff-duration
Aug 13, 2019
Merged

metric, tikv: record duration for each backoff type#11710
sre-bot merged 3 commits into
pingcap:masterfrom
lysu:dev/record-backoff-duration

Conversation

@lysu
Copy link
Copy Markdown
Contributor

@lysu lysu commented Aug 12, 2019

What problem does this PR solve?

current "KV Errors" - "KV Retry Duration" metrics in grafana only record coprocessor's backoff time.

that miss:

  • 2pc commit path's backoff
  • kv get path's backoff

it also miss the fine-gain time for each backoff-type

What is changed and how it works?

  • record backoff duration in backoff
  • remove duplicate cop duration recording

Check List

Tests

  • Unit test
  • Integration test
  • Manual test (add detailed scripts or steps below)

Code changes

  • change metric record

Side effects

  • N/A

Related changes

  • Need to cherry-pick to the release branch

This change is Reviewable

@lysu
Copy link
Copy Markdown
Contributor Author

lysu commented Aug 12, 2019

/run-all-tests

@lysu lysu requested review from tiancaiamao and zz-jason August 12, 2019 03:38
@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 12, 2019

Codecov Report

Merging #11710 into master will increase coverage by 0.0318%.
The diff coverage is 85.7142%.

@@               Coverage Diff                @@
##             master     #11710        +/-   ##
================================================
+ Coverage   81.4159%   81.4478%   +0.0318%     
================================================
  Files           432        432                
  Lines         93080      93089         +9     
================================================
+ Hits          75782      75819        +37     
+ Misses        11876      11855        -21     
+ Partials       5422       5415         -7

@codecov
Copy link
Copy Markdown

codecov Bot commented Aug 12, 2019

Codecov Report

Merging #11710 into master will decrease coverage by 0.012%.
The diff coverage is 85.7142%.

@@               Coverage Diff                @@
##             master     #11710        +/-   ##
================================================
- Coverage   81.4407%   81.4286%   -0.0121%     
================================================
  Files           429        429                
  Lines         92746      92632       -114     
================================================
- Hits          75533      75429       -104     
+ Misses        11821      11809        -12     
- Partials       5392       5394         +2

Comment thread metrics/metrics.go Outdated
Comment thread store/tikv/backoff.go Outdated
Comment thread store/tikv/backoff.go Outdated
@lysu
Copy link
Copy Markdown
Contributor Author

lysu commented Aug 13, 2019

@tiancaiamao @zz-jason PTAL

Copy link
Copy Markdown
Member

@zz-jason zz-jason left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@zz-jason zz-jason added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 13, 2019
@tiancaiamao
Copy link
Copy Markdown
Contributor

LGTM

@tiancaiamao tiancaiamao added status/LGT2 Indicates that a PR has LGTM 2. and removed status/LGT1 Indicates that a PR has LGTM 1. labels Aug 13, 2019
@tiancaiamao tiancaiamao added the status/can-merge Indicates a PR has been approved by a committer. label Aug 13, 2019
@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Aug 13, 2019

/run-all-tests

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Aug 13, 2019

cherry pick to release-3.0 in PR #11728

@sre-bot
Copy link
Copy Markdown
Contributor

sre-bot commented Aug 13, 2019

cherry pick to release-2.1 failed

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

component/metrics status/can-merge Indicates a PR has been approved by a committer. status/LGT2 Indicates that a PR has LGTM 2.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants