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

docs(configuration): add cluster configuration #851

Merged
merged 2 commits into from
Jun 18, 2014

Conversation

yichengq
Copy link
Contributor

@yichengq
Copy link
Contributor Author

@robszumski @colegleason @xiangli-cmu

@@ -41,6 +41,9 @@ configuration files.
* `-peer-election-timeout` - The number of milliseconds to wait before the leader is declared unhealthy.
* `-peer-heartbeat-interval` - The number of milliseconds in between heartbeat requests
* `-snapshot=false` - Disable log snapshots. Defaults to `true`.
* `-cluster-active-size` - The number of expected peer-mode instances in the cluster. It is used only when the instance creates a new cluster.
Copy link
Contributor

Choose a reason for hiding this comment

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

It is used only when the instance creates a new cluster.

Is there a better way to explain this? Basically it sets the value in the cluster and then the cluster value is used from then onwards?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It sets the value if it decides to build a one-node cluster by itself, and does it in the init step.
The cluster uses this value from then onwards.
If user wants to change it later, they could only do it through HTTP API currently.

Copy link
Contributor

Choose a reason for hiding this comment

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

OK. What about:

"Only applied if the etcd instance is the first peer in the cluster."

I think we also need a section of this that talks about and links to the cluster config API similar to my other PR: https://github.com/coreos/etcd/pull/850/files#diff-c2e93b5d997618cfe14f6ea88f78f0d3R15

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agree with you.
Your PR is also on this file, so I would rely on that one to make it.

Copy link
Contributor

Choose a reason for hiding this comment

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

Whoops, I didn't realize you wanted to keep both issues.

@yichengq
Copy link
Contributor Author

@robszumski

@@ -41,6 +41,9 @@ configuration files.
* `-peer-election-timeout` - The number of milliseconds to wait before the leader is declared unhealthy.
* `-peer-heartbeat-interval` - The number of milliseconds in between heartbeat requests
* `-snapshot=false` - Disable log snapshots. Defaults to `true`.
* `-cluster-active-size` - The number of expected peer-mode instances in the cluster. Only applied if the etcd instance is the first peer 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.

The last nit I have on this is that we don't use the term peer-mode very often. When we explain this to be people verbally we say something about how it participates or not in the consensus protocol. Can we mention that here?

"The expected number of instances participating in the consensus protocol. Only applied if the etcd instance is the first peer in the cluster."

@yichengq
Copy link
Contributor Author

@robszumski
Thanks for the advice.
Review it again?

@robszumski
Copy link
Contributor

lgtm. Can you look over #850 again?

robszumski added a commit that referenced this pull request Jun 18, 2014
docs(configuration): add cluster configuration
@robszumski robszumski merged commit e9a21dd into etcd-io:master Jun 18, 2014
@yichengq yichengq deleted the 103 branch December 7, 2014 17:20
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