[
https://issues.apache.org/jira/browse/ZOOKEEPER-1411?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13234141#comment-13234141
]
Rakesh R commented on ZOOKEEPER-1411:
-------------------------------------
Hi Alex, the patch looks good to me. I have couple of suggestions/imprv points.
Please go through the following:
+Comment1+
IMO, in QPC.writeMembership(), before going to upgrade the old config file,
good to check the status of the membershipFilename operations. If any exception
occurred during the creation/writing of the membershipFilename, make more sense
to discontinue with the upgradation rather than modifying the cfg file, this
would cause inconsistent config and affect ZK startup/restart.
+Comment2+
{noformat}
QuorumPeer.java
try {
electionAddr = makeAddr(serverParts[0], Integer.parseInt(serverParts[2]));
} catch (NumberFormatException e) {
setType(serverParts[2]);
}
if (serverParts.length == 3) return;
if (electionAddr == null) {
throw new ConfigException(addressStr + wrongFormat);
}
{noformat}
Say, occurred NumberFormatException and 'serverParts.length == 3' is satisfied,
in this case it will continue to next level with electionAddr as null. IMO,
would throw ConfigException for NumberFormatException.
Hope the combination 'host:port:type' is newly adding to the code base as part
of this patch.
As I know, UDP electionAlg is not in a consistent state and also deprecated
this feature. Also in the QPC, there is a check for validating the electionAddr
- "Missing election port for server: ". So I feel, its not necessary to
consider new combination 'host:port:type' :-)
+Comment3+
QuorumPeer.java
public String membershipFilename = null;
public String configFilename = null;
public boolean membershipBackwardCompatibility = false;
Is it OK to have private scope?
+Comment4+
Improve the code format, please adjust it a bit by removing the 'tab spaces'
> Consolidate membership management and add client port information
> -----------------------------------------------------------------
>
> Key: ZOOKEEPER-1411
> URL: https://issues.apache.org/jira/browse/ZOOKEEPER-1411
> Project: ZooKeeper
> Issue Type: Sub-task
> Components: server
> Reporter: Alexander Shraer
> Assignee: Alexander Shraer
> Fix For: 3.5.0
>
> Attachments: ZOOKEEPER-1411-ver1.patch, ZOOKEEPER-1411-ver2.patch,
> ZOOKEEPER-1411-ver3.patch, ZOOKEEPER-1411-ver4.patch,
> ZOOKEEPER-1411-ver5.patch
>
>
> Currently every server has a different configuration file. With this patch,
> we will have all cluster membership definitions in a single file, and every
> sever can have a copy of this file.
> This also solves ZOOKEEPER-1113
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators:
https://issues.apache.org/jira/secure/ContactAdministrators!default.jspa
For more information on JIRA, see: http://www.atlassian.com/software/jira
