*: expose Leadership Transfer API to clients #8153

gyuho · 2017-06-21T23:20:43Z

heyitsanthony · 2017-06-22T16:45:25Z

etcdserver/api/v3rpc/rpctypes/error.go

@@ -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")


s/not a leader/not leader

heyitsanthony · 2017-06-22T16:45:35Z

etcdserver/errors.go

@@ -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")


s/not a leader/not leader/

heyitsanthony · 2017-06-22T16:46:46Z

etcdserver/etcdserverpb/rpc.proto

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

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


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

?

heyitsanthony · 2017-06-22T16:48:15Z

etcdserver/etcdserverpb/rpc.proto

@@ -191,6 +191,14 @@ service Maintenance {
        body: "*"
    };
  }
+
+  // TransferLeadership requests current leader node to transfer its leadership to transferee.


MoveLeader? TransferLeadership is kind of wordy

heyitsanthony · 2017-06-22T16:52:54Z

integration/v3_leadership_test.go

+	}
+}
+
+// TestTransferLeaderServiceNonLeader ensures that request to non-leader fail.


ensures followers return an error for transferring leadership.

heyitsanthony · 2017-06-22T16:54:45Z

etcdserver/api/v3rpc/maintenance.go

@@ -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)


return nil, ErrGRPCNotLeader?

heyitsanthony · 2017-06-22T16:57:07Z

etcdserver/api/v3rpc/maintenance.go

+
+	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)


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

heyitsanthony · 2017-06-22T17:00:20Z

etcdctl/ctlv3/command/transfer_leadership_command.go

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


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

heyitsanthony · 2017-06-22T17:00:34Z

etcdctl/ctlv3/command/transfer_leadership_command.go

+	cancel()
+	if cerr != nil {
+		ExitWithError(ExitError, cerr)
+		return


not needed since ExitWithError never returns

heyitsanthony · 2017-06-22T17:02:10Z

etcdctl/ctlv3/command/transfer_leadership_command.go

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


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

heyitsanthony · 2017-06-23T16:42:14Z

etcdctl/ctlv3/command/printer.go

@@ -43,6 +43,7 @@ type printer interface {
 	MemberList(v3.MemberListResponse)

 	EndpointStatus([]epStatus)
+	MoveLeader(leader, target uint64)


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

heyitsanthony · 2017-06-23T16:43:21Z

etcdctl/ctlv3/command/move_leader_command.go

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


extra err check?

heyitsanthony · 2017-06-23T16:44:08Z

etcdctl/ctlv3/command/move_leader_command.go

+
+		if resp.Header.GetMemberId() == resp.Leader {
+			leaderCli = cli
+			leaderID = resp.Leader


heyitsanthony · 2017-06-23T16:44:24Z

etcdctl/ctlv3/command/move_leader_command.go

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


extra err check?

heyitsanthony · 2017-06-23T16:48:26Z

etcdctl/README.md

@@ -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.


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

heyitsanthony · 2017-06-23T16:52:56Z

etcdserver/etcdserverpb/rpc.proto

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

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


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

heyitsanthony · 2017-06-23T16:55:35Z

etcdctl/ctlv3/command/move_leader_command.go

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


transferee-member-id?

heyitsanthony · 2017-06-23T16:55:53Z

etcdctl/ctlv3/command/move_leader_command.go

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


Transfers leadership to another etcd cluster member.

Signed-off-by: Gyu-Ho Lee <[email protected]>

See: etcd-io/etcd#8153

gyuho added the kind/release-note label Jun 21, 2017

gyuho added this to the v3.3.0-maybe milestone Jun 21, 2017

gyuho requested review from xiang90, fanminshi and heyitsanthony June 21, 2017 23:20

heyitsanthony suggested changes Jun 22, 2017

View reviewed changes

gyuho added the WIP label Jun 22, 2017

gyuho force-pushed the leadership-transfer branch from 9a7a9f4 to 27a260c Compare June 22, 2017 20:02

gyuho removed the WIP label Jun 22, 2017

gyuho force-pushed the leadership-transfer branch 2 times, most recently from 9f1a8d4 to 976b49f Compare June 23, 2017 00:29

heyitsanthony suggested changes Jun 23, 2017

View reviewed changes

gyuho added 10 commits June 23, 2017 12:43

etcdserver/etcdserverpb: define 'MoveLeader' RPC

d82f257

Signed-off-by: Gyu-Ho Lee <[email protected]>

*: regenerate proto with 'MoveLeader' RPC

265303c

Signed-off-by: Gyu-Ho Lee <[email protected]>

etcdserver/*: add 'ErrNotLeader'

939bbd7

Signed-off-by: Gyu-Ho Lee <[email protected]>

etcdserver/api/v3rpc: add 'MoveLeader' to 'maintenanceServer'

b1a0ae3

Signed-off-by: Gyu-Ho Lee <[email protected]>

proxy/*: add 'MoveLeader' RPC

3e263d5

Signed-off-by: Gyu-Ho Lee <[email protected]>

etcdserver: expose 'transferLeadership' as 'MoveLeader'

403ba1d

Signed-off-by: Gyu-Ho Lee <[email protected]>

integration: test 'MoveLeader' service

c5532dd

Signed-off-by: Gyu-Ho Lee <[email protected]>

clientv3/*: add 'MoveLeader' method to 'Maintenance'

581a83d

Signed-off-by: Gyu-Ho Lee <[email protected]>

etcdctl/ctlv3: add 'move-leader' command

6e9b776

Signed-off-by: Gyu-Ho Lee <[email protected]>

e2e: test 'move-leader' command

d428958

Signed-off-by: Gyu-Ho Lee <[email protected]>

gyuho force-pushed the leadership-transfer branch from 976b49f to d428958 Compare June 23, 2017 20:20

heyitsanthony approved these changes Jul 6, 2017

View reviewed changes

gyuho merged commit a57405a into etcd-io:master Jul 6, 2017

gyuho deleted the leadership-transfer branch July 6, 2017 20:00

etcd-io deleted a comment from codecov-io Jul 7, 2017

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

etcd: expose transferLeadership.

c448dcc

See: etcd-io/etcd#8153

disksing added a commit to oh-my-tidb/pd that referenced this pull request Jan 30, 2018

etcd: expose transferLeadership.

7b87a51

See: etcd-io/etcd#8153

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

*: expose Leadership Transfer API to clients #8153

*: expose Leadership Transfer API to clients #8153

gyuho commented Jun 21, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 22, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

heyitsanthony Jun 23, 2017

*: expose Leadership Transfer API to clients #8153

*: expose Leadership Transfer API to clients #8153

Conversation

gyuho commented Jun 21, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment