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

*: support raft learner in etcd - part 1 #10725

Merged
merged 9 commits into from
May 15, 2019
Merged

*: support raft learner in etcd - part 1 #10725

merged 9 commits into from
May 15, 2019

Conversation

jingyih
Copy link
Contributor

@jingyih jingyih commented May 14, 2019

The first 7 commits of #10645. Rebased to master.

cc @xiang90

@jingyih
*: Change etcdserver API to support raft learner
7dc5451 
- Added isLearner flag to MemberAddRequest in Cluster API.
- Added isLearner field to StatusResponse in Maintenance API.
- Added MemberPromote rpc to Cluster API.
@jingyih
*: fix compilation after API change
a0d3c4d 
Fixed compilation erros after API change for learner.
@jingyih
etcdserver: support MemberAdd for learner
604bc04 
Added IsLearner field to etcdserver internal Member type. Routed
learner MemberAdd request from server API to raft. Apply learner
MemberAdd result to server after the request is passed through Raft.
@jingyih
clientv3: support MemberAdd for learner
fc14608 
Added IsLearner flag to clientv3 MemberAdd API.
@jingyih
etcdctl: support MemberAdd for learner
a67d934 
Added support for "etcdctl member add --learner".
@jingyih
tests/e2e: Add test for learner member add
e4296bb 
Added an e2e test to exercise "etcdctl member add --learner".
@jingyih
etcdctl: add learner field in member list output
1e38de5
@jingyih jingyih mentioned this pull request May 14, 2019
Closed
xiang90
xiang90 reviewed May 14, 2019
View reviewed changes
clientv3/cluster.go Outdated Show resolved Hide resolved
@xiang90
Copy link
Contributor

xiang90 commented May 14, 2019

lgtm after improving the interface thing.

@xiang90
Copy link
Contributor

xiang90 commented May 14, 2019

build failed with

clientv3/integration/cluster_test.go:80:25: not enough arguments in call to capi.Cluster.MemberAdd
	have (context.Context, []string)
	want (context.Context, []string, bool)
FAIL	go.etcd.io/etcd/v3/clientv3/integration [build failed]

@jingyih
Copy link
Contributor Author

jingyih commented May 14, 2019

build failed with

clientv3/integration/cluster_test.go:80:25: not enough arguments in call to capi.Cluster.MemberAdd
	have (context.Context, []string)
	want (context.Context, []string, bool)
FAIL	go.etcd.io/etcd/v3/clientv3/integration [build failed]

Yeah. That is known. There are conflicts to master branch due to changes added to master branch after learner feature branch's branching off. They are fixed after a later rebasing. This particular one is fixed in 11e4168.

@jingyih
Copy link
Contributor Author

jingyih commented May 14, 2019

There are also conflicts in functional tests, which are fixed in c8fbc4d.

Since we are going to have a new function MemberAddAsLearner, these conflicts will be fixed.

@jingyih
Copy link
Contributor Author

jingyih commented May 14, 2019 •
edited
Loading

@xiang90 BTW, we had travis CI enabled in learner feature branch during the development. So all tests will pass if all commits are included. When this PR is created, I rebased to latest master, therefore the conflicts - maybe we should get the whole thing reviewed and merge together?

@jingyih
*: add MemberAddAsLearner to clientv3 Cluster API
2b76200 
Made changes to Clientv3 Cluster API:

- Added MemberAddAsLearner.
- Reverted changes to MemberAdd - removed input parameter isLearner.
xiang90
xiang90 reviewed May 15, 2019
View reviewed changes
proxy/grpcproxy/cluster.go Outdated Show resolved Hide resolved
@xiang90
Copy link
Contributor

xiang90 commented May 15, 2019

Lgtm after fixing the nits

@jingyih
etcdserver: Add MemberAddAsLearner
e1acf24 
Made changes to api/membership:

- Added MemberAddAsLearner
- Reverted changes to MemberAdd - removed input parameter isLearner
@xiang90
Copy link
Contributor

xiang90 commented May 15, 2019

Let us merge the commits in pr by pr.

@jingyih
Copy link
Contributor Author

jingyih commented May 15, 2019

Let us merge the commits in pr by pr.

Can you clarify?

@xiang90
Copy link
Contributor

xiang90 commented May 15, 2019

@jingyih Let us split #10645 to multiple PRs and merge them one by one. This is the first one that we will merge soon.

@jingyih
Copy link
Contributor Author

jingyih commented May 15, 2019

@jingyih Let us split #10645 to multiple PRs and merge them one by one. This is the first one that we will merge soon.

Got it.

@xiang90 xiang90 merged commit 919b93b into etcd-io:master May 15, 2019
@gyuho
Copy link
Contributor

gyuho commented May 15, 2019

@jingyih Can you highlight those changes from this PR in our changelog? Thanks for the great work!

@jingyih
Copy link
Contributor Author

jingyih commented May 15, 2019

@jingyih Can you highlight those changes from this PR in our changelog? Thanks for the great work!

Sure I will update the changelog.

@jingyih jingyih mentioned this pull request May 15, 2019
Merged
@jingyih jingyih mentioned this pull request May 29, 2019
Merged
@jingyih jingyih mentioned this pull request Jun 9, 2019
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@xiang90 xiang90 xiang90 left review comments

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jingyih @xiang90 @gyuho