-
Notifications
You must be signed in to change notification settings - Fork 505
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
Etcd based cluster config #382
Etcd based cluster config #382
Conversation
Codecov Report
@@ Coverage Diff @@
## main #382 +/- ##
==========================================
+ Coverage 80.57% 80.82% +0.24%
==========================================
Files 60 60
Lines 6888 6961 +73
==========================================
+ Hits 5550 5626 +76
- Misses 1043 1044 +1
+ Partials 295 291 -4
Continue to review full report at Codecov.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please also fix the issues reported by Github actions.
…and commanline arguments for alternative cluster start-up routine.
02ad79d
to
beb28c3
Compare
I don't go into detail about the code in the first review, I would like to specify my vision about this PR along with reasons.
Primary member: name: primary-1
cluster-name: cluster-test
cluster-role: primary
cluster-primary:
listen-peer-urls: [http://127.0.0.1:2380]
listen-client-urls: [http://127.0.0.1:2379]
advertise-client-urls: [http://127.0.0.1:2379]
initial-advertise-peer-urls: [http://127.0.0.1:2380]
initial-cluster-state: new
initial-cluster:
- primary-1: http://127.0.0.1:2380
- primary-2: http://127.0.0.1:2378
- primary-3: http://127.0.0.1:2376 Secondary member: name: secondary-1
cluster-name: cluster-test
cluster-role: secondary
cluster-secondary:
primary-listen-peer-urls: [http://127.0.0.1:2380] The reason I designed them like that is to make it cleaner to write/read config by splitting them up.
|
@xxx7xxxx Thanks for the comment! I think that the example you proposed makes it simpler to add new reader/secondary members. I would develop it further like this: Almost the same config for primary member. name: primary-1
cluster-name: cluster-test
cluster-role: primary # writer
cluster: # only primary member defines cluster entry
listen-peer-urls: [http://127.0.0.1:2380]
listen-client-urls: [http://127.0.0.1:2379]
advertise-client-urls: [http://127.0.0.1:2379]
initial-advertise-peer-urls: [http://127.0.0.1:2380]
initial-cluster-state: new
initial-cluster:
- primary-1: http://127.0.0.1:2380
- primary-2: http://127.0.0.1:2378
- primary-3: http://127.0.0.1:2376 and then secondary name: secondary-1
cluster-name: cluster-test
cluster-role: secondary # reader
# secondary member defines, where to connect to primary
primary-listen-peer-urls: [http://127.0.0.1:2380] |
@xxx7xxxx I removed the extra logic from members.go, so it's easier to remove in future. The configuration is also as described above. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
- As I commented last time, please adapt older examples of reader-004/reader-005.
- Have you tested adding members, then shutdown all members, then restart all nodes, will it restart successfully?
…se legacy cluster startup routine. Remove update fnctions from pkg/option/option
Close #233. Based on this design #233 (comment).