Skip to content

Commit

Permalink
placement: fix placement location issue (#2603) (#2605)
Browse files Browse the repository at this point in the history 
Signed-off-by: disksing <i@disksing.com>
  • Loading branch information
@ti-srebot
ti-srebot authored Jul 3, 2020
1 parent db0aa7e commit 1309eaf
Show file tree
Hide file tree
Showing 3 changed files with 126 additions and 69 deletions.
24 changes: 24 additions & 0 deletions server/schedule/checker/rule_checker_test.go
View file
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
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
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)
}
}

0 comments on commit 1309eaf

Please sign in to comment.