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

placement: fix placement location issue (#2603) #2605

Merged
merged 3 commits into from
Jul 3, 2020
Merged

placement: fix placement location issue (#2603) #2605

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
41baa42
placement: fix placement location issue
disksing Jul 1, 2020
6f811d5
add tests
disksing Jul 1, 2020
201155a
add isolationScore test
disksing Jul 2, 2020
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
24 changes: 24 additions & 0 deletions server/schedule/checker/rule_checker_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,30 @@ func (s *testRuleCheckerSuite) TestBetterReplacement(c *C) {
c.Assert(op, IsNil)
}

func (s *testRuleCheckerSuite) TestBetterReplacement2(c *C) {
s.cluster.AddLabelsStore(1, 1, map[string]string{"zone": "z1", "host": "host1"})
s.cluster.AddLabelsStore(2, 1, map[string]string{"zone": "z1", "host": "host2"})
s.cluster.AddLabelsStore(3, 1, map[string]string{"zone": "z1", "host": "host3"})
s.cluster.AddLabelsStore(4, 1, map[string]string{"zone": "z2", "host": "host1"})
s.cluster.AddLeaderRegionWithRange(1, "", "", 1, 2, 3)
s.ruleManager.SetRule(&placement.Rule{
GroupID: "pd",
ID: "test",
Index: 100,
Override: true,
Role: placement.Voter,
Count: 3,
LocationLabels: []string{"zone", "host"},
})
op := s.rc.Check(s.cluster.GetRegion(1))
c.Assert(op, NotNil)
c.Assert(op.Desc(), Equals, "move-to-better-location")
c.Assert(op.Step(0).(operator.AddLearner).ToStore, Equals, uint64(4))
s.cluster.AddLeaderRegionWithRange(1, "", "", 1, 3, 4)
op = s.rc.Check(s.cluster.GetRegion(1))
c.Assert(op, IsNil)
}

func (s *testRuleCheckerSuite) TestNoBetterReplacement(c *C) {
s.cluster.AddLabelsStore(1, 1, map[string]string{"host": "host1"})
s.cluster.AddLabelsStore(2, 1, map[string]string{"host": "host1"})
Expand Down
47 changes: 22 additions & 25 deletions server/schedule/placement/fit.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
package placement

import (
"math"
"sort"

"github.com/pingcap/kvproto/pkg/metapb"
Expand Down Expand Up @@ -85,9 +86,9 @@ type RuleFit struct {
// different Role from configuration (the Role can be migrated to target role
// by scheduling).
PeersWithDifferentRole []*metapb.Peer
// IsolationLevel indicates at which level of labeling these Peers are
// isolated. A larger value indicates a higher isolation level.
IsolationLevel int
// IsolationScore indicates at which level of labeling these Peers are
// isolated. A larger value is better.
IsolationScore float64
}

// IsSatisfied returns if the rule is properly satisfied.
Expand All @@ -105,9 +106,9 @@ func compareRuleFit(a, b *RuleFit) int {
return -1
case len(a.PeersWithDifferentRole) < len(b.PeersWithDifferentRole):
return 1
case a.IsolationLevel < b.IsolationLevel:
case a.IsolationScore < b.IsolationScore:
return -1
case a.IsolationLevel > b.IsolationLevel:
case a.IsolationScore > b.IsolationScore:
return 1
default:
return 0
Expand Down Expand Up @@ -159,7 +160,7 @@ func fitRule(peers []*fitPeer, rule *Rule) *RuleFit {
}

func newRuleFit(rule *Rule, peers []*fitPeer) *RuleFit {
rf := &RuleFit{Rule: rule, IsolationLevel: isolationLevel(peers, rule.LocationLabels)}
rf := &RuleFit{Rule: rule, IsolationScore: isolationScore(peers, rule.LocationLabels)}
for _, p := range peers {
rf.Peers = append(rf.Peers, p.Peer)
if !p.matchRoleStrict(rule.Role) {
Expand Down Expand Up @@ -236,28 +237,24 @@ func iterPeersRecr(peers []*fitPeer, index int, out []*fitPeer, f func()) {
}
}

func isolationLevel(peers []*fitPeer, labels []string) int {
if len(labels) == 0 || len(peers) == 0 {
func isolationScore(peers []*fitPeer, labels []string) float64 {
var score float64
if len(labels) == 0 || len(peers) <= 1 {
return 0
}
if len(peers) == 1 {
return len(labels)
}
if len(peers) == 2 {
for l, label := range labels {
if peers[0].store.GetLabelValue(label) != peers[1].store.GetLabelValue(label) {
return len(labels) - l
// NOTE: following loop is partially duplicated with `core.DistinctScore`.
// The reason not to call it directly is that core.DistinctScore only
// accepts `[]StoreInfo` not `[]*fitPeer` and I don't want alloc slice
// here because it is kind of hot path.
// After Go supports generics, we will be enable to do some refactor and
// reuse `core.DistinctScore`.
const replicaBaseScore = 100
for i, p1 := range peers {
for _, p2 := range peers[i+1:] {
if index := p1.store.CompareLocation(p2.store, labels); index != -1 {
score += math.Pow(replicaBaseScore, float64(len(labels)-index-1))
}
}
return 0
}

// TODO: brute force can be improved.
level := len(labels)
iterPeers(peers, 2, func(pair []*fitPeer) {
if l := isolationLevel(pair, labels); l < level {
level = l
}
})
return level
return score
}
124 changes: 80 additions & 44 deletions server/schedule/placement/fit_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var _ = Suite(&testFitSuite{})

type testFitSuite struct{}

func (s *testFitSuite) TestFitByLocation(c *C) {
func (s *testFitSuite) makeStores() map[uint64]*core.StoreInfo {
stores := make(map[uint64]*core.StoreInfo)
for zone := 1; zone <= 5; zone++ {
for rack := 1; rack <= 5; rack++ {
Expand All @@ -44,6 +44,11 @@ func (s *testFitSuite) TestFitByLocation(c *C) {
}
}
}
return stores
}

func (s *testFitSuite) TestFitByLocation(c *C) {
stores := s.makeStores()

type Case struct {
// peers info
Expand All @@ -54,8 +59,7 @@ func (s *testFitSuite) TestFitByLocation(c *C) {
count int // default: len(peerStoreID)
role PeerRoleType // default: Voter
// expect result:
expectedPeers []uint64 // default: same as peerStoreID
expectedIsolationLevel int // default: 0
expectedPeers []uint64 // default: same as peerStoreID
}

cases := []Case{
Expand All @@ -65,59 +69,59 @@ func (s *testFitSuite) TestFitByLocation(c *C) {
{peerStoreID: []uint64{1111, 1112, 1113}, count: 3, expectedPeers: []uint64{1111, 1112, 1113}},
{peerStoreID: []uint64{1111, 1112, 1113}, count: 5, expectedPeers: []uint64{1111, 1112, 1113}},
// test isolation level
{peerStoreID: []uint64{1111}, locationLabels: "zone,rack,host", expectedIsolationLevel: 3},
{peerStoreID: []uint64{1111}, locationLabels: "zone,rack", expectedIsolationLevel: 2},
{peerStoreID: []uint64{1111}, locationLabels: "zone", expectedIsolationLevel: 1},
{peerStoreID: []uint64{1111}, locationLabels: "", expectedIsolationLevel: 0},
{peerStoreID: []uint64{1111, 2111}, locationLabels: "zone,rack,host", expectedIsolationLevel: 3},
{peerStoreID: []uint64{1111, 2222, 3333}, locationLabels: "zone,rack,host", expectedIsolationLevel: 3},
{peerStoreID: []uint64{1111, 1211, 3111}, locationLabels: "zone,rack,host", expectedIsolationLevel: 2},
{peerStoreID: []uint64{1111, 1121, 3111}, locationLabels: "zone,rack,host", expectedIsolationLevel: 1},
{peerStoreID: []uint64{1111, 1121, 1122}, locationLabels: "zone,rack,host", expectedIsolationLevel: 0},
{peerStoreID: []uint64{1111}, locationLabels: "zone,rack,host"},
{peerStoreID: []uint64{1111}, locationLabels: "zone,rack"},
{peerStoreID: []uint64{1111}, locationLabels: "zone"},
{peerStoreID: []uint64{1111}, locationLabels: ""},
{peerStoreID: []uint64{1111, 2111}, locationLabels: "zone,rack,host"},
{peerStoreID: []uint64{1111, 2222, 3333}, locationLabels: "zone,rack,host"},
{peerStoreID: []uint64{1111, 1211, 3111}, locationLabels: "zone,rack,host"},
{peerStoreID: []uint64{1111, 1121, 3111}, locationLabels: "zone,rack,host"},
{peerStoreID: []uint64{1111, 1121, 1122}, locationLabels: "zone,rack,host"},
// test best location
{
peerStoreID: []uint64{1111, 1112, 1113, 2111, 2222, 3222, 3333},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 2111, 3222},
expectedIsolationLevel: 3,
peerStoreID: []uint64{1111, 1112, 1113, 2111, 2222, 3222, 3333},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 2111, 3222},
},
{
peerStoreID: []uint64{1111, 1121, 1211, 2111, 2211},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 1211, 2111},
expectedIsolationLevel: 2,
peerStoreID: []uint64{1111, 1121, 1211, 2111, 2211},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 1211, 2111},
},
{
peerStoreID: []uint64{1111, 1211, 1311, 1411, 2111, 2211, 2311, 3111},
locationLabels: "zone,rack,host",
count: 5,
expectedPeers: []uint64{1111, 1211, 2111, 2211, 3111},
},
// test role match
{
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 1,
expectedPeers: []uint64{1112},
expectedIsolationLevel: 0,
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 1,
expectedPeers: []uint64{1112},
},
{
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 2,
expectedPeers: []uint64{1112, 1113},
expectedIsolationLevel: 0,
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 2,
expectedPeers: []uint64{1112, 1113},
},
{
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 3,
expectedPeers: []uint64{1112, 1113, 1111},
expectedIsolationLevel: 0,
peerStoreID: []uint64{1111, 1112, 1113},
peerRole: []PeerRoleType{Learner, Follower, Follower},
count: 3,
expectedPeers: []uint64{1112, 1113, 1111},
},
{
peerStoreID: []uint64{1111, 1112, 1121, 1122, 1131, 1132, 1141, 1142},
peerRole: []PeerRoleType{Follower, Learner, Learner, Learner, Learner, Follower, Follower, Follower},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 1132, 1141},
expectedIsolationLevel: 1,
peerStoreID: []uint64{1111, 1112, 1121, 1122, 1131, 1132, 1141, 1142},
peerRole: []PeerRoleType{Follower, Learner, Learner, Learner, Learner, Follower, Follower, Follower},
locationLabels: "zone,rack,host",
count: 3,
expectedPeers: []uint64{1111, 1132, 1141},
},
}

Expand Down Expand Up @@ -159,6 +163,38 @@ func (s *testFitSuite) TestFitByLocation(c *C) {
}
sort.Slice(expectedPeers, func(i, j int) bool { return expectedPeers[i] < expectedPeers[j] })
c.Assert(selectedIDs, DeepEquals, expectedPeers)
c.Assert(ruleFit.IsolationLevel, Equals, cc.expectedIsolationLevel)
}
}

func (s *testFitSuite) TestIsolationScore(c *C) {
stores := s.makeStores()
testCases := []struct {
peers1 []uint64
Checker
peers2 []uint64
}{
{[]uint64{1111, 1112}, Less, []uint64{1111, 1121}},
{[]uint64{1111, 1211}, Less, []uint64{1111, 2111}},
{[]uint64{1111, 1211, 1311, 2111, 3111}, Less, []uint64{1111, 1211, 2111, 2211, 3111}},
{[]uint64{1111, 1211, 2111, 2211, 3111}, Equals, []uint64{1111, 2111, 2211, 3111, 3211}},
{[]uint64{1111, 1211, 2111, 2211, 3111}, Greater, []uint64{1111, 1121, 2111, 2211, 3111}},
}

makePeers := func(ids []uint64) []*fitPeer {
var peers []*fitPeer
for _, id := range ids {
peers = append(peers, &fitPeer{
Peer: &metapb.Peer{StoreId: id},
store: stores[id],
})
}
return peers
}

for _, tc := range testCases {
peers1, peers2 := makePeers(tc.peers1), makePeers(tc.peers2)
score1 := isolationScore(peers1, []string{"zone", "rack", "host"})
score2 := isolationScore(peers2, []string{"zone", "rack", "host"})
c.Assert(score1, tc.Checker, score2)
}
}