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

*: expose Leadership Transfer API to clients #8153

Merged
merged 10 commits into from
Jul 6, 2017

Conversation

gyuho
Copy link
Contributor

@gyuho gyuho commented Jun 21, 2017

Fix #7584.

@gyuho gyuho added this to the v3.3.0-maybe milestone Jun 21, 2017
@@ -59,6 +59,7 @@ var (
ErrGRPCInvalidAuthMgmt = grpc.Errorf(codes.InvalidArgument, "etcdserver: invalid auth management")

ErrGRPCNoLeader = grpc.Errorf(codes.Unavailable, "etcdserver: no leader")
ErrGRPCNotLeader = grpc.Errorf(codes.Unavailable, "etcdserver: not a leader")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/not a leader/not leader

@@ -29,6 +29,7 @@ var (
ErrTimeoutLeaderTransfer = errors.New("etcdserver: request timed out, leader transfer took too long")
ErrNotEnoughStartedMembers = errors.New("etcdserver: re-configuration failed due to not enough started members")
ErrNoLeader = errors.New("etcdserver: no leader")
ErrNotLeader = errors.New("etcdserver: not a leader")
Copy link
Contributor

Choose a reason for hiding this comment

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

s/not a leader/not leader/

@@ -785,6 +793,15 @@ message DefragmentResponse {
ResponseHeader header = 1;
}

message TransferLeadershipRequest {
// transferee is the target node ID for leadership transfer.
uint64 transferee = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

// target is the node ID for the new leader.
uint64 target = 1;

?

@@ -191,6 +191,14 @@ service Maintenance {
body: "*"
};
}

// TransferLeadership requests current leader node to transfer its leadership to transferee.
Copy link
Contributor

Choose a reason for hiding this comment

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

MoveLeader? TransferLeadership is kind of wordy

}
}

// TestTransferLeaderServiceNonLeader ensures that request to non-leader fail.
Copy link
Contributor

Choose a reason for hiding this comment

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

ensures followers return an error for transferring leadership.

@@ -147,6 +153,20 @@ func (ms *maintenanceServer) Status(ctx context.Context, ar *pb.StatusRequest) (
return resp, nil
}

func (ms *maintenanceServer) TransferLeadership(ctx context.Context, tr *pb.TransferLeadershipRequest) (*pb.TransferLeadershipResponse, error) {
if ms.rg.ID() != ms.rg.Leader() {
return nil, togRPCError(etcdserver.ErrNotLeader)
Copy link
Contributor

Choose a reason for hiding this comment

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

return nil, ErrGRPCNotLeader?


plog.Noticef("starting to transfer leadership from %s to %s", ms.rg.ID(), types.ID(tr.Transferee))
if err := ms.lt.TransferLeader(ctx, uint64(ms.rg.Leader()), tr.Transferee); err != nil {
return nil, togRPCError(err)
Copy link
Contributor

Choose a reason for hiding this comment

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

is there already a warning or error message if there's a failure to transfer? the 'starting' message should probably be matched with some completion message even if it fails

ExitWithError(ExitError, cerr)
return
}
fmt.Printf("leader transferred to %s\n", types.ID(transferee))
Copy link
Contributor

Choose a reason for hiding this comment

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

this should use the printer/display stuff since it maps directly to an RPC

cancel()
if cerr != nil {
ExitWithError(ExitError, cerr)
return
Copy link
Contributor

Choose a reason for hiding this comment

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

not needed since ExitWithError never returns

if len(args) != 1 {
ExitWithError(ExitBadArgs, fmt.Errorf("transfer-leadership command needs 1 argument"))
}

Copy link
Contributor

Choose a reason for hiding this comment

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

can this figure out the current leader instead of expecting the user to supply the leader's endpoint?

@gyuho gyuho added the WIP label Jun 22, 2017
@gyuho gyuho removed the WIP label Jun 22, 2017
@gyuho gyuho force-pushed the leadership-transfer branch 2 times, most recently from 9f1a8d4 to 976b49f Compare June 23, 2017 00:29
@@ -43,6 +43,7 @@ type printer interface {
MemberList(v3.MemberListResponse)

EndpointStatus([]epStatus)
MoveLeader(leader, target uint64)
Copy link
Contributor

Choose a reason for hiding this comment

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

since this is an RPC, it should probably be MoveLeader(leader, target uint64, v3.MoveLeaderResponse) and support printerRPC

DialTimeout: 3 * time.Second,
})
if err != nil {
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

extra err check?


if resp.Header.GetMemberId() == resp.Leader {
leaderCli = cli
leaderID = resp.Leader
Copy link
Contributor

Choose a reason for hiding this comment

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

break?

}
}
resp, err := cli.Status(ctx, ep)
if err != nil {
Copy link
Contributor

Choose a reason for hiding this comment

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

extra err check?

@@ -805,6 +805,29 @@ Prints a line of JSON encoding the database hash, revision, total keys, and size
+----------+----------+------------+------------+
```

### MOVE-LEADER \<hexadecimal-transferee-id\>

MOVE-LEADER manually transfers a leadership to another node in the cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

MOVE-LEADER transfers leadership from the leader to another member in the cluster.

@@ -787,6 +795,15 @@ message DefragmentResponse {
ResponseHeader header = 1;
}

message MoveLeaderRequest {
// target is the node ID for the new leader.
uint64 target = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

judging by the rest of rpc.proto, this probably needs ID in the name to be consistent with other rpc message naming-- something like targetID, memberID, or ID

// NewMoveLeaderCommand returns the cobra command for "move-leader".
func NewMoveLeaderCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "move-leader <transferee>",
Copy link
Contributor

Choose a reason for hiding this comment

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

transferee-member-id?

func NewMoveLeaderCommand() *cobra.Command {
cmd := &cobra.Command{
Use: "move-leader <transferee>",
Short: "Transfers leadership.",
Copy link
Contributor

Choose a reason for hiding this comment

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

Transfers leadership to another etcd cluster member.

gyuho added 10 commits June 23, 2017 12:43
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
Signed-off-by: Gyu-Ho Lee <gyuhox@gmail.com>
@gyuho gyuho merged commit a57405a into etcd-io:master Jul 6, 2017
@gyuho gyuho deleted the leadership-transfer branch July 6, 2017 20:00
@etcd-io etcd-io deleted a comment from codecov-io Jul 7, 2017
@gyuho gyuho removed this from the v3.3.0-maybe milestone Nov 21, 2017
disksing added a commit to oh-my-tidb/pd that referenced this pull request Jan 29, 2018
disksing added a commit to oh-my-tidb/pd that referenced this pull request Jan 30, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants