Skip to content
This repository has been archived by the owner on Nov 24, 2023. It is now read-only.

feature: remove metrics when stopping task #575

Merged
merged 15 commits into from
Apr 8, 2020
Merged

feature: remove metrics when stopping task #575

merged 15 commits into from
Apr 8, 2020

Conversation

Kuri-su
Copy link
Contributor

@Kuri-su Kuri-su commented Apr 1, 2020 •
edited by csuzhangxc
Loading

UCP #503

What problem does this PR solve?

fix #503

What is changed and how it works?

  • add pkg 'metricsProxy'
    • it will note labels list when operate metrics
  • add some code to clear metrics labels value in 'Close' function in these Struct
    • SubTask
    • Dumpling
    • Loader
    • Syncer
    • Mydumper

@Kuri-su
Copy link
Contributor Author

Kuri-su commented Apr 1, 2020 •
edited
Loading

Maybe i need to add unit tests in this pkg

Done

@Kuri-su
Copy link
Contributor Author

Kuri-su commented Apr 1, 2020

@csuzhangxc PTAL 😄

@sre-bot
Copy link

sre-bot commented Apr 2, 2020

Thanks for your contribution. If your PR get merged, you will be rewarded 300 points.

@csuzhangxc csuzhangxc added the status/PTAL This PR is ready for review. Add this label back after committing new changes label Apr 2, 2020
@csuzhangxc
Copy link
Member

/run-all-tests

@codecov
Copy link

codecov bot commented Apr 2, 2020 •
edited
Loading

Codecov Report

Merging #575 into master will not change coverage by %.
The diff coverage is n/a.

@@             Coverage Diff             @@
##             master       #575   +/-   ##
===========================================
  Coverage   57.8183%   57.8183%           
===========================================
  Files           200        200           
  Lines         21085      21085           
===========================================
  Hits          12191      12191           
  Misses         7702       7702           
  Partials       1192       1192

@csuzhangxc csuzhangxc removed the status/PTAL This PR is ready for review. Add this label back after committing new changes label Apr 2, 2020
@csuzhangxc
Copy link
Member

/run-all-tests

@Kuri-su Kuri-su force-pushed the feature/kurisu-remove-metrics-when-stopping-task-200320 branch from eff6aaa to 5a2a3a1 Compare April 2, 2020 17:17
@Kuri-su Kuri-su changed the title [WIP] feature: remove metrics when stopping task feature: remove metrics when stopping task Apr 2, 2020
@Kuri-su
Copy link
Contributor Author

Kuri-su commented Apr 2, 2020

@csuzhangxc I'am Done , PTAL 😄

@Kuri-su
Copy link
Contributor Author

Kuri-su commented Apr 2, 2020

/run-all-tests

1 similar comment
@lichunzhu
Copy link
Contributor

/run-all-tests

@Kuri-su Kuri-su closed this Apr 7, 2020
@Kuri-su Kuri-su reopened this Apr 7, 2020
lichunzhu
lichunzhu reviewed Apr 7, 2020
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

Rest LGTM

pkg/metricsproxy/histogramVec_test.go Outdated Show resolved Hide resolved
csuzhangxc
csuzhangxc reviewed Apr 7, 2020
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

rest LGTM, cool.

pkg/metricsproxy/counterVec.go Outdated Show resolved Hide resolved
pkg/metricsproxy/proxy.go Outdated Show resolved Hide resolved
syncer/syncer.go Outdated Show resolved Hide resolved
@csuzhangxc
Copy link
Member

/award-point 600

@sre-bot
Copy link

sre-bot commented Apr 7, 2020

Not allowed to award more than 500 points.

@csuzhangxc
Copy link
Member

/award-point 500

@sre-bot
Copy link

sre-bot commented Apr 7, 2020

Update score success, the task will rewarded 500 after merged.

@Kuri-su Kuri-su force-pushed the feature/kurisu-remove-metrics-when-stopping-task-200320 branch from e0bf2dd to f5f2939 Compare April 7, 2020 15:29
@Kuri-su Kuri-su requested a review from csuzhangxc April 7, 2020 17:05
csuzhangxc
csuzhangxc reviewed Apr 8, 2020
View reviewed changes
Copy link
Member

@csuzhangxc csuzhangxc left a comment

Choose a reason for hiding this comment

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

LGTM

@csuzhangxc csuzhangxc added needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT1 One reviewer already commented LGTM type/feature New feature labels Apr 8, 2020
lichunzhu
lichunzhu approved these changes Apr 8, 2020
View reviewed changes
Copy link
Contributor

@lichunzhu lichunzhu left a comment

Choose a reason for hiding this comment

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

Great job! LGTM.

@lichunzhu lichunzhu merged commit 6f1549f into pingcap:master Apr 8, 2020
@sre-bot
Copy link

sre-bot commented Apr 8, 2020

Team □□□□□ complete task #503 and get 500 score, currerent score 750

@sre-bot
Copy link

sre-bot commented Apr 8, 2020

cherry pick to release-1.0 failed

csuzhangxc pushed a commit to csuzhangxc/dm that referenced this pull request Apr 17, 2020
@Kuri-su @csuzhangxc
feature: remove metrics when stopping task (pingcap#575)
a25ad08
@csuzhangxc csuzhangxc mentioned this pull request Apr 17, 2020
Merged
@csuzhangxc csuzhangxc added already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked status/LGT2 Two reviewers already commented LGTM, ready for merge and removed needs-cherry-pick-release-1.0 This PR should be cherry-picked to release-1.0. Remove this label after cherry-picked to release-1.0 status/LGT1 One reviewer already commented LGTM labels Apr 17, 2020
lichunzhu added a commit that referenced this pull request Apr 17, 2020
@csuzhangxc @Kuri-su @lichunzhu
cherry-pick #575 and #578 to release-1.0 and resolve conflicts (#616)
7dec1e5 
* feature:  remove metrics when stopping task  (#575)

* .*/: fix dm unit tests and integration tests (#578)

* tests: fix merge

Co-authored-by: Amatist_Kurisu <[email protected]>
Co-authored-by: Chunzhu Li <[email protected]>
@csuzhangxc csuzhangxc mentioned this pull request Apr 30, 2020
Closed
@csuzhangxc csuzhangxc added already-update-release-note The release note is updated. Add this label once the release note is updated and removed needs-update-release-note This PR should be added into release notes. Remove this label once the release notes are updated labels Jun 17, 2020
@GMHDBJD GMHDBJD mentioned this pull request Sep 6, 2022
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@csuzhangxc csuzhangxc csuzhangxc left review comments

@lichunzhu lichunzhu lichunzhu approved these changes

Assignees
No one assigned
Labels
already-cherry-pick-1.0 The related PR is already cherry-picked to release-1.0. Add this label once the PR is cherry-picked already-update-release-note The release note is updated. Add this label once the release note is updated contribution For contributor priority/normal Minor change, requires approval from ≥1 primary reviewer status/LGT2 Two reviewers already commented LGTM, ready for merge type/feature New feature
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

UCP: remove metrics when stopping the task
4 participants
@Kuri-su @sre-bot @csuzhangxc @lichunzhu