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

server: minor changes for add/remove peer. #106

Merged
merged 1 commit into from
May 20, 2016
Merged
Changes from all commits
Commits
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
66 changes: 16 additions & 50 deletions server/cluster_worker.go
Original file line number Diff line number Diff line change
Expand Up @@ -40,71 +40,37 @@ func (c *raftCluster) handleAddPeerReq(region *metapb.Region) (*metapb.Peer, err
// check this.
// 2, We can check the store statistics and find a low load store.
// 3, more algorithms...
var matchStore *metapb.Store
L:
for _, store := range mu.stores {
storeID := store.GetId()

existStore := false
// we can't add peer in the same store.
for _, peer := range region.Peers {
if peer.GetStoreId() == storeID {
// we can't add peer in the same store.
existStore = true
break
if peer.GetStoreId() == store.GetId() {
continue L
}
}

if existStore {
continue
}

matchStore = &store

break
}

if matchStore == nil {
return nil, errors.Errorf("find no store to add peer for region %v", region)
}

peer := &metapb.Peer{
StoreId: proto.Uint64(matchStore.GetId()),
Id: proto.Uint64(peerID),
return &metapb.Peer{
Id: proto.Uint64(peerID),
StoreId: proto.Uint64(store.GetId()),
}, nil
}

region.Peers = append(region.Peers, peer)

return peer, nil
return nil, errors.Errorf("find no store to add peer for region %v", region)
}

// If leader is nil, we will return an error, or else we can remove none leader peer.
func (c *raftCluster) handleRemovePeerReq(region *metapb.Region, leader *metapb.Peer) (*metapb.Peer, error) {
func (c *raftCluster) handleRemovePeerReq(region *metapb.Region, leaderID uint64) (*metapb.Peer, error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we check the validity of leaderID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@qiuyesuifeng I think it's acceptable not to check it. And if we want to, maybe better to check it outside of this func.

if len(region.Peers) <= 1 {
return nil, errors.Errorf("can not remove peer for region %v", region)
}
if leader == nil {
return nil, errors.Errorf("invalid leader for region %v", region)
}

found, idx := false, 0
for i, peer := range region.Peers {
if peer.GetId() != leader.GetId() {
found = true
idx = i
break
for _, peer := range region.Peers {
if peer.GetId() != leaderID {
return peer, nil
}
}

if found {
peer := region.Peers[idx]
region.Peers = append(region.Peers[:idx], region.Peers[idx+1:]...)
return peer, nil
}

// Maybe we can't enter here.
return nil, errors.Errorf("find no proper peer to remove for region %v", region)
}

func (c *raftCluster) handleChangePeerReq(region *metapb.Region, leader *metapb.Peer) (*pdpb.ChangePeer, error) {
func (c *raftCluster) handleChangePeerReq(region *metapb.Region, leaderID uint64) (*pdpb.ChangePeer, error) {
clusterMeta, err := c.GetConfig()
if err != nil {
return nil, errors.Trace(err)
Expand All @@ -130,7 +96,7 @@ func (c *raftCluster) handleChangePeerReq(region *metapb.Region, leader *metapb.
} else {
log.Infof("region %d peer number %d > %d, need to remove peer", regionID, peerNumber, maxPeerNumber)
changeType = raftpb.ConfChangeType_RemoveNode
if peer, err = c.handleRemovePeerReq(region, leader); err != nil {
if peer, err = c.handleRemovePeerReq(region, leaderID); err != nil {
return nil, errors.Trace(err)
}
}
Expand Down Expand Up @@ -170,7 +136,7 @@ func (c *raftCluster) maybeChangePeer(request *pdpb.RegionHeartbeatRequest, reqR
}

// If the request epoch configure version is equal to the current one, handle change peer request.
changePeer, err := c.handleChangePeerReq(reqRegion, leader)
changePeer, err := c.handleChangePeerReq(reqRegion, leader.GetId())
if err != nil {
return nil, nil, errors.Trace(err)
}
Expand Down