You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by maoling <gi...@git.apache.org> on 2018/09/14 03:40:55 UTC
[GitHub] zookeeper pull request #624: ZOOKEEPER-3108:use a new property server.id in ...
GitHub user maoling opened a pull request:
https://github.com/apache/zookeeper/pull/624
ZOOKEEPER-3108:use a new property server.id in the zoo.cfg to substitute for myid file
- use a new property `server.id` in the `zoo.cfg` to substitute for `myid `file,If the unique id is both set in the `server.id` of `zoo.cfg` and `myid` file,the `server.id` has the priority.
- I had tested this patch in my docker in standalone and qurom mode with mixed `myid` file and the `server.id`
it has no intrude,totally backwards compatibility,branch3.4 can still use myid file, but branch 3.5+,
suggest to use `server.id `in the zoo.cfg
- the related UTs `ZooKeeperServerMainTest.testStandalone()` and `QuorumPeerMainTest.testQuorum() `have passed,before reviewing,let's listen to the QA report in case of something I miss
- more details and disscusion in [ZOOKEEPER-3108](https://issues.apache.org/jira/browse/ZOOKEEPER-3108)
- Let me list why this is needed:
**1.** put the `myid` which's about the conf of server in the `dataDir` is not a good idea.if I want to `rm -rf `the data in this directory manually,I will delete the `myid` file by mistake.
**2.** conf like this will be easy-to-read,and tell me who I am just like `kafka` does.
```
server.id = 2
---*******---
server.1=172.17.0.2:2888:3888
server.2=172.17.0.2:12888:13888
server.3=172.17.0.2:22888:23888
```
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/maoling/zookeeper ZOOKEEPER-3108
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/zookeeper/pull/624.patch
To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:
This closes #624
----
commit af2a12c6e4037cf0010ea15c43355ec4f5ea4318
Author: maoling <ma...@...>
Date: 2018-09-05T08:44:54Z
ZOOKEEPER-3108:use a new property server.id in the zoo.cfg to substitute for myid file
----
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/624
Previously, it was not part of the dynamic config, because it’s in a separate config file. But after this change it will move to the dynamic config, if I remember correctly it check and move all server. prefix to dynamic config file, but maybe I remember it wrongly.
Anyway, we need a test case to cover that.
---
[GitHub] zookeeper pull request #624: ZOOKEEPER-3108:use a new property server.id in ...
Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/624#discussion_r217873955
--- Diff: zookeeper-docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
@@ -265,18 +266,25 @@ server.3=zoo3:2888:3888</programlisting>
role="bold">server.id=host:port:port</emphasis>. The parameters <emphasis
role="bold">host</emphasis> and <emphasis
role="bold">port</emphasis> are straightforward. You attribute the
- server id to each machine by creating a file named
+ server id to each machine by setting <emphasis role="bold">server.id</emphasis>
+ to a unique integer for each zookeeper server.To keep backwards compatibility,
+ you can still creat a file named
--- End diff --
we should also talk about the importance of not changing the id since it becomes much easier with the configuration file option. we should have documented that anyway, but it becomes much more important with this change since the id is decoupled from the data.
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/624
@lvfangmin
read the [doc](http://zookeeper.apache.org/doc/r3.5.1-alpha/zookeeperReconfig.html) and the code in the `QuorumPeerConfig.parseDynamicConfig()`,IMO,the` server.id` is only to be one of the static config and not included into the dynamic config file.WDYT?
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by maoling <gi...@git.apache.org>.
Github user maoling commented on the issue:
https://github.com/apache/zookeeper/pull/624
@Thanks all for reviewing,I will deal with this suggestions ASAP.
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by breed <gi...@git.apache.org>.
Github user breed commented on the issue:
https://github.com/apache/zookeeper/pull/624
thanks for remembering to update the doc @maoling !
---
[GitHub] zookeeper pull request #624: ZOOKEEPER-3108:use a new property server.id in ...
Posted by eolivelli <gi...@git.apache.org>.
Github user eolivelli commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/624#discussion_r217763082
--- Diff: src/java/main/org/apache/zookeeper/server/quorum/flexible/QuorumMaj.java ---
@@ -83,7 +83,7 @@ public QuorumMaj(Properties props) throws ConfigException {
String key = entry.getKey().toString();
String value = entry.getValue().toString();
- if (key.startsWith("server.")) {
+ if (key.startsWith("server.") && !key.startsWith("server.id")) {
--- End diff --
Maybe key.equals(server.id)
---
[GitHub] zookeeper pull request #624: ZOOKEEPER-3108:use a new property server.id in ...
Posted by enixon <gi...@git.apache.org>.
Github user enixon commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/624#discussion_r217812851
--- Diff: zookeeper-docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
@@ -265,18 +266,25 @@ server.3=zoo3:2888:3888</programlisting>
role="bold">server.id=host:port:port</emphasis>. The parameters <emphasis
role="bold">host</emphasis> and <emphasis
role="bold">port</emphasis> are straightforward. You attribute the
- server id to each machine by creating a file named
+ server id to each machine by setting <emphasis role="bold">server.id</emphasis>
+ to a unique integer for each zookeeper server.To keep backwards compatibility,
+ you can still creat a file named
--- End diff --
nit - spelling 'create'
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit commented on the issue:
https://github.com/apache/zookeeper/pull/624
Refer to this link for build results (access rights to CI server needed):
https://builds.apache.org/job/PreCommit-ZOOKEEPER-github-pr-build/2305/
---
[GitHub] zookeeper issue #624: ZOOKEEPER-3108:use a new property server.id in the zoo...
Posted by lvfangmin <gi...@git.apache.org>.
Github user lvfangmin commented on the issue:
https://github.com/apache/zookeeper/pull/624
The newly added server.id is supposed to one of the static config, right? If I read the patch correctly, seems it will be treated as dynamic config, and the server.id in dynamic config won't be read in the next startup.
Can you add some test case to cover this?
---
[GitHub] zookeeper pull request #624: ZOOKEEPER-3108:use a new property server.id in ...
Posted by breed <gi...@git.apache.org>.
Github user breed commented on a diff in the pull request:
https://github.com/apache/zookeeper/pull/624#discussion_r217873847
--- Diff: zookeeper-docs/src/documentation/content/xdocs/zookeeperAdmin.xml ---
@@ -265,18 +266,25 @@ server.3=zoo3:2888:3888</programlisting>
role="bold">server.id=host:port:port</emphasis>. The parameters <emphasis
role="bold">host</emphasis> and <emphasis
role="bold">port</emphasis> are straightforward. You attribute the
- server id to each machine by creating a file named
+ server id to each machine by setting <emphasis role="bold">server.id</emphasis>
+ to a unique integer for each zookeeper server.To keep backwards compatibility,
+ you can still creat a file named
<filename>myid</filename>, one for each server, which resides in
that server's data directory, as specified by the configuration file
- parameter <emphasis role="bold">dataDir</emphasis>.</para></listitem>
+ parameter <emphasis role="bold">dataDir</emphasis>.If the unique id is both set in the
+ server.id of zoo.cfg and myid file,the server.id has the priority
--- End diff --
i think we should return an error in this case. this will make debugging problems that result from this situation hard.
---