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.

Sign up for GitHub

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

Jump to bottom

rule_checker: can replace unhealthPeer with orphanPeer #6831

Merged
merged 10 commits into from
Jul 26, 2023
Merged

rule_checker: can replace unhealthPeer with orphanPeer #6831

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
cf1d5ac
rule_checker: can replace unhealthPeer with orphanPeer
nolouch Jul 21, 2023
b3e3e00
Merge branch 'master' into fix-unhealthy-repair
nolouch Jul 21, 2023
eb8251f
fix test
nolouch Jul 24, 2023
ee4b85e
Merge remote-tracking branch 'nolouch/fix-unhealthy-repair' into fix-…
nolouch Jul 24, 2023
4ab99b1
address
nolouch Jul 25, 2023
bbbeea7
address
nolouch Jul 25, 2023
65aa679
address
nolouch Jul 26, 2023
d7c092d
address
nolouch Jul 26, 2023
b72fbea
address
nolouch Jul 26, 2023
a148eda
Merge branch 'master' into fix-unhealthy-repair
ti-chi-bot[bot] Jul 26, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
fix test 
Signed-off-by: nolouch <nolouch@gmail.com>
  • Loading branch information
@nolouch
nolouch committed Jul 24, 2023
commit eb8251fe11d659224089157deb0822856a889c71
3 changes: 2 additions & 1 deletion pkg/schedule/checker/rule_checker.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -480,11 +480,12 @@ loopFits:
continue
}
// check if down peer can replace with orphan peer.
ruleCheckerReplaceOrphanPeerCounter.Inc()

dstStore := c.cluster.GetStore(orphanPeer.GetStoreId())
if fit.Replace(pinDownPeer.GetPeer().GetStoreId(), dstStore) {
Copy link
Contributor

Choose a reason for hiding this comment

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

waht it will do if the orphan peer is also down peer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we will skip in the before.

destRole := pinDownPeer.GetPeer().Role
Copy link
Member

@rleungx rleungx Jul 26, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

How about using a variable for pinDownPeer.GetPeer() and using GetRole here?

orphanPeerRole := orphanPeer.GetRole()
ruleCheckerReplaceOrphanPeerCounter.Inc()
switch {
case orphanPeerRole == metapb.PeerRole_Learner && destRole == metapb.PeerRole_Voter:
return operator.CreatePromoteLearnerOperatorAndRemovePeer("replace-down-peer-with-orphan-peer", c.cluster, region, orphanPeer, pinDownPeer.GetPeer())
Expand Down
28 changes: 17 additions & 11 deletions server/api/operator_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -217,7 +217,13 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
regionURL := fmt.Sprintf("%s/operators/%d", suite.urlPrefix, region.GetId())
operator := mustReadURL(re, regionURL)
suite.Contains(operator, "operator not found")

convertStepsToStr := func(steps []string) string {
stepStrs := make([]string, len(steps))
for i := range steps {
stepStrs[i] = fmt.Sprintf("step-%d:{%s}", i, steps[i])
}
return strings.Join(stepStrs, ", ")
}
testCases := []struct {
name string
placementRuleEnable bool
Expand All @@ -231,25 +237,25 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
placementRuleEnable: false,
input: []byte(`{"name":"transfer-region", "region_id": 1, "to_store_ids": [2, 3]}`),
expectedError: nil,
expectSteps: strings.Join([]string{
expectSteps: convertStepsToStr([]string{
pdoperator.AddLearner{ToStore: 3, PeerID: 1}.String(),
pdoperator.PromoteLearner{ToStore: 3, PeerID: 1}.String(),
pdoperator.TransferLeader{FromStore: 1, ToStore: 2}.String(),
pdoperator.RemovePeer{FromStore: 1, PeerID: 1}.String(),
}, ", "),
}),
},
{
name: "placement rule disable with peer role",
placementRuleEnable: false,
input: []byte(`{"name":"transfer-region", "region_id": 1, "to_store_ids": [2, 3], "peer_roles":["follower", "leader"]}`),
expectedError: nil,
expectSteps: strings.Join([]string{
expectSteps: convertStepsToStr([]string{
pdoperator.AddLearner{ToStore: 3, PeerID: 2}.String(),
pdoperator.PromoteLearner{ToStore: 3, PeerID: 2}.String(),
pdoperator.TransferLeader{FromStore: 1, ToStore: 2}.String(),
pdoperator.RemovePeer{FromStore: 1, PeerID: 2}.String(),
pdoperator.TransferLeader{FromStore: 2, ToStore: 3}.String(),
}, ", "),
}),
},
{
name: "default placement rule without peer role",
Expand All @@ -262,13 +268,13 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
name: "default placement rule with peer role",
placementRuleEnable: true,
input: []byte(`{"name":"transfer-region", "region_id": 1, "to_store_ids": [2, 3], "peer_roles":["follower", "leader"]}`),
expectSteps: strings.Join([]string{
expectSteps: convertStepsToStr([]string{
pdoperator.AddLearner{ToStore: 3, PeerID: 3}.String(),
pdoperator.PromoteLearner{ToStore: 3, PeerID: 3}.String(),
pdoperator.TransferLeader{FromStore: 1, ToStore: 2}.String(),
pdoperator.RemovePeer{FromStore: 1, PeerID: 1}.String(),
pdoperator.TransferLeader{FromStore: 2, ToStore: 3}.String(),
}, ", "),
}),
},
{
name: "default placement rule with invalid input",
Expand Down Expand Up @@ -323,12 +329,12 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
},
input: []byte(`{"name":"transfer-region", "region_id": 1, "to_store_ids": [2, 3], "peer_roles":["follower", "leader"]}`),
expectedError: nil,
expectSteps: strings.Join([]string{
expectSteps: convertStepsToStr([]string{
pdoperator.AddLearner{ToStore: 3, PeerID: 5}.String(),
pdoperator.PromoteLearner{ToStore: 3, PeerID: 5}.String(),
pdoperator.TransferLeader{FromStore: 1, ToStore: 3}.String(),
pdoperator.RemovePeer{FromStore: 1, PeerID: 1}.String(),
}, ", "),
}),
},
{
name: "customized placement rule with valid peer role2",
Expand Down Expand Up @@ -363,12 +369,12 @@ func (suite *transferRegionOperatorTestSuite) TestTransferRegionWithPlacementRul
},
input: []byte(`{"name":"transfer-region", "region_id": 1, "to_store_ids": [2, 3], "peer_roles":["leader", "follower"]}`),
expectedError: nil,
expectSteps: strings.Join([]string{
expectSteps: convertStepsToStr([]string{
pdoperator.AddLearner{ToStore: 3, PeerID: 6}.String(),
pdoperator.PromoteLearner{ToStore: 3, PeerID: 6}.String(),
pdoperator.TransferLeader{FromStore: 1, ToStore: 2}.String(),
pdoperator.RemovePeer{FromStore: 1, PeerID: 1}.String(),
}, ", "),
}),
},
}
for _, testCase := range testCases {
Expand Down