Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

cluster: restart prometheus after scaling the cluster #717

Merged
merged 4 commits into from
Aug 27, 2020

Conversation

AstroProfundis
Copy link
Contributor

What problem does this PR solve?

The prometheus config is not reloaded correctly after scaling in a cluster.

What is changed and how it works?

Restart the prometheus instance after updating configs, otherwise new configs are not applied.

Check List

Tests

  • Integration test

Code changes

  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity

Related changes

  • Need to cherry-pick to the release branch

Release notes:

NONE

@AstroProfundis AstroProfundis added type/bug-fix Categorizes PR as a bug-fix category/stability Categorizes issue or PR as a stability enhancement. labels Aug 27, 2020
@AstroProfundis AstroProfundis added this to the v1.1.0 milestone Aug 27, 2020
@AstroProfundis AstroProfundis self-assigned this Aug 27, 2020
Copy link
Contributor

@lonng lonng left a comment

Choose a reason for hiding this comment

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

LGTM

@ti-srebot ti-srebot added the status/LGT1 Indicates that a PR has LGTM 1. label Aug 27, 2020
@lonng
Copy link
Contributor

lonng commented Aug 27, 2020

/merge

@ti-srebot ti-srebot added the status/can-merge Indicates a PR has been approved by a committer. label Aug 27, 2020
@ti-srebot
Copy link
Contributor

Your auto merge job has been accepted, waiting for:

  • 695
  • 707

@codecov-commenter
Copy link

codecov-commenter commented Aug 27, 2020

Codecov Report

Merging #717 into master will increase coverage by 0.03%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #717      +/-   ##
==========================================
+ Coverage   57.93%   57.97%   +0.03%     
==========================================
  Files         253      253              
  Lines       18522    18530       +8     
==========================================
+ Hits        10731    10743      +12     
+ Misses       6367     6365       -2     
+ Partials     1424     1422       -2     
Flag Coverage Δ
#coverage 57.97% <ø> (+0.03%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...c/github.com/pingcap/tiup/pkg/cluster/api/dmapi.go 62.77% <0.00%> (ø)
...src/github.com/pingcap/tiup/pkg/cluster/manager.go 69.00% <0.00%> (+0.15%) ⬆️
...com/pingcap/tiup/components/dm/command/scale_in.go 69.41% <0.00%> (+0.36%) ⬆️
...ingcap/tiup/components/cluster/command/scale_in.go 82.35% <0.00%> (+0.53%) ⬆️
...c/github.com/pingcap/tiup/pkg/cluster/api/pdapi.go 57.01% <0.00%> (+1.21%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e8a7b45...d16f7ad. Read the comment docs.

@ti-srebot
Copy link
Contributor

/run-all-tests

@ti-srebot
Copy link
Contributor

@AstroProfundis merge failed.

@july2993 july2993 merged commit 4978e39 into pingcap:master Aug 27, 2020
@AstroProfundis AstroProfundis deleted the fix-scale-in branch January 19, 2021 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category/stability Categorizes issue or PR as a stability enhancement. status/can-merge Indicates a PR has been approved by a committer. status/LGT1 Indicates that a PR has LGTM 1. type/bug-fix Categorizes PR as a bug-fix
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants