Skip to content

Conversation

@9547
Copy link
Contributor

@9547 9547 commented Dec 13, 2020

What problem does this PR solve?

fix #944

What is changed and how it works?

Check List

Tests

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

Code changes

  • Has exported function/method change
  • Has exported variable/fields change
  • Has interface methods change
  • Has persistent data change

Side effects

  • Possible performance regression
  • Increased code complexity
  • Breaking backward compatibility

Related changes

  • Need to cherry-pick to the release branch
  • Need to update the documentation

Release notes:

NONE

@9547 9547 marked this pull request as draft December 13, 2020 10:12
@ti-chi-bot ti-chi-bot added the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 13, 2020
@ti-chi-bot ti-chi-bot requested review from breezewish and nrc December 13, 2020 10:12
@ti-chi-bot ti-chi-bot added the size/S Denotes a PR that changes 10-29 lines, ignoring generated files. label Dec 13, 2020
@9547 9547 changed the title typo(cluster): nodesID -> nodesIDs Feature: don't reload prometheus if the scaled comp is itself Dec 13, 2020
@codecov-io
Copy link

codecov-io commented Dec 13, 2020

Codecov Report

Merging #989 (1022823) into master (7668e42) will decrease coverage by 6.87%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #989      +/-   ##
==========================================
- Coverage   55.48%   48.60%   -6.88%     
==========================================
  Files         279      279              
  Lines       19710    19719       +9     
==========================================
- Hits        10936     9585    -1351     
- Misses       7061     8618    +1557     
+ Partials     1713     1516     -197     
Flag Coverage Δ
cluster 31.75% <100.00%> (-11.59%) ⬇️
dm 23.91% <45.00%> (+0.07%) ⬆️
integrate 42.06% <100.00%> (-7.69%) ⬇️
playground 20.28% <ø> (ø)
tiup 16.46% <ø> (ø)
unittest 23.05% <9.52%> (-0.01%) ⬇️

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

Impacted Files Coverage Δ
components/cluster/command/patch.go 50.00% <100.00%> (-20.00%) ⬇️
components/dm/command/patch.go 50.00% <100.00%> (ø)
pkg/cluster/manager/builder.go 82.19% <100.00%> (-3.01%) ⬇️
pkg/cluster/manager/destroy.go 64.78% <100.00%> (-4.33%) ⬇️
pkg/cluster/manager/scale_in.go 59.52% <100.00%> (ø)
pkg/cluster/task/builder.go 69.00% <100.00%> (-28.10%) ⬇️
pkg/cluster/task/update_meta.go 72.30% <100.00%> (-4.62%) ⬇️
pkg/cluster/task/update_topology.go 71.15% <100.00%> (+3.84%) ⬆️
pkg/cluster/template/config/config.go 0.00% <0.00%> (-100.00%) ⬇️
pkg/cluster/template/scripts/scripts.go 0.00% <0.00%> (-100.00%) ⬇️
... and 67 more

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 7668e42...1022823. Read the comment docs.

@9547 9547 force-pushed the feature/donot-reload-prom-if-scale-in-prom branch from 4853e46 to e197556 Compare December 14, 2020 16:43
@ti-chi-bot ti-chi-bot added size/M Denotes a PR that changes 30-99 lines, ignoring generated files. and removed size/S Denotes a PR that changes 10-29 lines, ignoring generated files. labels Dec 14, 2020
@9547 9547 marked this pull request as ready for review December 14, 2020 16:52
@ti-chi-bot ti-chi-bot removed the do-not-merge/work-in-progress Indicates that a PR should not merge because it is a work in progress. label Dec 14, 2020
@AstroProfundis AstroProfundis added category/stability Categorizes issue or PR as a stability enhancement. type/bug-fix Categorizes PR as a bug-fix labels Dec 15, 2020
@ti-chi-bot ti-chi-bot added the status/LGT1 Indicates that a PR has LGTM 1. label Dec 15, 2020
@AstroProfundis AstroProfundis added the category/monitoring Categorizes issue or PR related to monitoring components. label Dec 15, 2020
@lucklove
Copy link
Member

lucklove commented Dec 16, 2020

There are two test cases failed, both on scale-out promethues and waiting the instance number. I retried the test and it still failed, I think it's not a coincidence.

@lucklove
Copy link
Member

/merge

@ti-chi-bot ti-chi-bot added the status/can-merge Indicates a PR has been approved by a committer. label Dec 16, 2020
@ti-chi-bot
Copy link
Member

Can merge label has been added.

DetailsGit tree hash: 1022823

@ti-chi-bot ti-chi-bot merged commit d98eabc into pingcap:master Dec 16, 2020
@9547 9547 deleted the feature/donot-reload-prom-if-scale-in-prom branch April 6, 2021 23:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

category/monitoring Categorizes issue or PR related to monitoring components. category/stability Categorizes issue or PR as a stability enhancement. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. 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.

Don't need to reload Prometheus if the scale-in component is Prometheus

5 participants