symat commented on a change in pull request #1356:
URL: https://github.com/apache/zookeeper/pull/1356#discussion_r427212264
##########
File path:
zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Leader.java
##########
@@ -1508,20 +1510,25 @@ private synchronized void startZkServer() {
newLeaderProposal.ackSetsToString(),
Long.toHexString(zk.getZxid()));
- /*
- * ZOOKEEPER-1324. the leader sends the new config it must complete
- * to others inside a NEWLEADER message (see LearnerHandler where
- * the NEWLEADER message is constructed), and once it has enough
- * acks we must execute the following code so that it applies the
- * config to itself.
- */
- QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
+ if (QuorumPeerConfig.isReconfigEnabled()) {
+ /*
+ * ZOOKEEPER-1324. the leader sends the new config it must complete
+ * to others inside a NEWLEADER message (see LearnerHandler where
+ * the NEWLEADER message is constructed), and once it has enough
+ * acks we must execute the following code so that it applies the
+ * config to itself.
+ */
+ QuorumVerifier newQV = self.getLastSeenQuorumVerifier();
- Long designatedLeader = getDesignatedLeader(newLeaderProposal,
zk.getZxid());
+ Long designatedLeader = getDesignatedLeader(newLeaderProposal,
zk.getZxid());
- self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
- if (designatedLeader != self.getId()) {
- allowedToCommit = false;
+ self.processReconfig(newQV, designatedLeader, zk.getZxid(), true);
+ if (designatedLeader != self.getId()) {
+ LOG.warn("This leader is not the designated leader, it will be
initialized with allowedToCommit = false");
+ allowedToCommit = false;
+ }
+ } else {
+ LOG.debug("Reconfig feature is disabled, skip designatedLeader
calculation and reconfig processing.");
Review comment:
I agree with the LOG.info (this message will be printed only once when
the leader starts to lead after an election. Yet, the message can help
debugging production issues, when debug logs are disabled)
The tests are simulating the "backward compatibility" case when
`QuorumPeerConfig.isReconfigEnabled()==false`. I don't think that changing the
config with rolling-restart would be advised when dynamic-reconfig is enabled.
I think when the reconfig is enabled, the new config can propagate during the
leader election, so the rolling-restart will be unneccessary.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
