This is an automated email from the ASF dual-hosted git repository.

arshad pushed a commit to branch branch-3.7
in repository https://gitbox.apache.org/repos/asf/zookeeper.git


The following commit(s) were added to refs/heads/branch-3.7 by this push:
     new e2f3108  ZOOKEEPER-4257: learner.asyncSending, 
learner.closeSocketAsync and leader.closeSocketAsync should be configurable in 
zoo.cfg
e2f3108 is described below

commit e2f31086965f0d9d13898bbc72c57f378b438edf
Author: Ayush Mantri <[email protected]>
AuthorDate: Sun Mar 28 10:05:40 2021 +0530

    ZOOKEEPER-4257: learner.asyncSending, learner.closeSocketAsync and 
leader.closeSocketAsync should be configurable in zoo.cfg
    
    Properties learner.asyncSending , learner.closeSocketAsync  and 
leader.closeSocketAsync  made configurable in zoo.cfg
    
    Author: Ayush Mantri <[email protected]>
    
    Reviewers: Enrico Olivelli <[email protected]>, Mohammad Arshad 
<[email protected]>
    
    Closes #1645 from ayushmantri/ZK-4257 and squashes the following commits:
    
    8ff5523c3 [Ayush Mantri] ZOOKEEPER-4257: learner.asyncSending and 
learner.closeSocketAsync should be configurable in zoo.cfg                 
property name change done in backward compatible way
    592931232 [Ayush Mantri] ZOOKEEPER-4257: learner.asyncSending and 
learner.closeSocketAsync should be configurable in zoo.cfg
    
    (cherry picked from commit acbfb2d78996cbaa29b9dc764d4cb3463193dc45)
    Signed-off-by: Mohammad Arshad <[email protected]>
---
 .../src/main/resources/markdown/zookeeperAdmin.md  | 23 ++++++++----
 .../apache/zookeeper/server/quorum/Learner.java    | 11 +++---
 .../zookeeper/server/quorum/LearnerHandler.java    |  7 ++--
 .../apache/zookeeper/server/util/ConfigUtils.java  | 25 +++++++++++++
 .../zookeeper/server/util/ConfigUtilsTest.java     | 42 ++++++++++++++++++++++
 5 files changed, 96 insertions(+), 12 deletions(-)

diff --git a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md 
b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
index 1c59ce0..8334efd 100644
--- a/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
+++ b/zookeeper-docs/src/main/resources/markdown/zookeeperAdmin.md
@@ -1162,14 +1162,25 @@ property, when available, is noted below.
     When set to 0, no requests will be throttled. The default is 0.
 
 * *learner.closeSocketAsync*
-    (Jave system property only: **learner.closeSocketAsync**)
-    **New in 3.6.2:**
-    When enabled, a learner will close the quorum socket asynchronously. This 
is useful for TLS connections where closing a socket might take a long time, 
block the shutdown process, potentially delay a new leader election, and leave 
the quorum unavailabe. Closing the socket asynchronously avoids blocking the 
shutdown process despite the long socket closing time and a new leader election 
can be started while the socket being closed. The default is false.
+  (Java system property: **zookeeper.learner.closeSocketAsync**)
+  (Java system property: **learner.closeSocketAsync**)(Added for backward 
compatibility)
+    **New in 3.7.0:**
+    When enabled, a learner will close the quorum socket asynchronously. This 
is useful for TLS connections where closing a socket might take a long time, 
block the shutdown process, potentially delay a new leader election, and leave 
the quorum unavailabe. Closing the socket asynchronously avoids blocking the 
shutdown process despite the long socket closing time and a new leader election 
can be started while the socket being closed. 
+    The default is false.
 
 * *leader.closeSocketAsync*
-   (Java system property only: **leader.closeSocketAsync**)
-   **New in 3.6.2:**
-   When enabled, the leader will close a quorum socket asynchoronously. This 
is useful for TLS connections where closing a socket might take a long time. If 
disconnecting a follower is initiated in ping() because of a failed 
SyncLimitCheck then the long socket closing time will block the sending of 
pings to other followers. Without receiving pings, the other followers will not 
send session information to the leader, which causes sessions to expire. 
Setting this flag to true ensures that  [...]
+  (Java system property: **zookeeper.leader.closeSocketAsync**)
+  (Java system property: **leader.closeSocketAsync**)(Added for backward 
compatibility)
+   **New in 3.7.0:**
+   When enabled, the leader will close a quorum socket asynchoronously. This 
is useful for TLS connections where closing a socket might take a long time. If 
disconnecting a follower is initiated in ping() because of a failed 
SyncLimitCheck then the long socket closing time will block the sending of 
pings to other followers. Without receiving pings, the other followers will not 
send session information to the leader, which causes sessions to expire. 
Setting this flag to true ensures that  [...]
+   The default is false.
+
+* *learner.asyncSending*
+  (Java system property: **zookeeper.learner.asyncSending**)
+  (Java system property: **learner.asyncSending**)(Added for backward 
compatibility)
+  **New in 3.7.0:**
+  The sending and receiving packets in Learner were done synchronously in a 
critical section. An untimely network issue could cause the followers to hang 
(see [ZOOKEEPER-3575](https://issues.apache.org/jira/browse/ZOOKEEPER-3575) and 
[ZOOKEEPER-4074](https://issues.apache.org/jira/browse/ZOOKEEPER-4074)). The 
new design moves sending packets in Learner to a separate thread and sends the 
packets asynchronously. The new design is enabled with this parameter 
(learner.asyncSending).
+  The default is false.
 
 * *forward_learner_requests_to_commit_processor_disabled*
     (Jave system property: 
**zookeeper.forward_learner_requests_to_commit_processor_disabled**)
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
index 8f3d00c..594c87f 100644
--- 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/Learner.java
@@ -58,6 +58,7 @@ import org.apache.zookeeper.server.TxnLogEntry;
 import org.apache.zookeeper.server.ZooTrace;
 import org.apache.zookeeper.server.quorum.QuorumPeer.QuorumServer;
 import org.apache.zookeeper.server.quorum.flexible.QuorumVerifier;
+import org.apache.zookeeper.server.util.ConfigUtils;
 import org.apache.zookeeper.server.util.MessageTracker;
 import org.apache.zookeeper.server.util.SerializeUtils;
 import org.apache.zookeeper.server.util.ZxidUtils;
@@ -118,10 +119,12 @@ public class Learner {
 
     private static final boolean nodelay = 
System.getProperty("follower.nodelay", "true").equals("true");
 
-    public static final String LEARNER_ASYNC_SENDING = "learner.asyncSending";
-    private static boolean asyncSending = 
Boolean.getBoolean(LEARNER_ASYNC_SENDING);
-    public static final String LEARNER_CLOSE_SOCKET_ASYNC = 
"learner.closeSocketAsync";
-    public static final boolean closeSocketAsync = 
Boolean.getBoolean(LEARNER_CLOSE_SOCKET_ASYNC);
+    public static final String LEARNER_ASYNC_SENDING = 
"zookeeper.learner.asyncSending";
+    private static boolean asyncSending =
+        
Boolean.parseBoolean(ConfigUtils.getPropertyBackwardCompatibleWay(LEARNER_ASYNC_SENDING));
+    public static final String LEARNER_CLOSE_SOCKET_ASYNC = 
"zookeeper.learner.closeSocketAsync";
+    public static final boolean closeSocketAsync = Boolean
+        
.parseBoolean(ConfigUtils.getPropertyBackwardCompatibleWay(LEARNER_CLOSE_SOCKET_ASYNC));
 
     static {
         LOG.info("leaderConnectDelayDuringRetryMs: {}", 
leaderConnectDelayDuringRetryMs);
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerHandler.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerHandler.java
index b91319e..735f370 100644
--- 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerHandler.java
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/quorum/LearnerHandler.java
@@ -51,6 +51,7 @@ import org.apache.zookeeper.server.ZooTrace;
 import org.apache.zookeeper.server.quorum.Leader.Proposal;
 import org.apache.zookeeper.server.quorum.QuorumPeer.LearnerType;
 import org.apache.zookeeper.server.quorum.auth.QuorumAuthServer;
+import org.apache.zookeeper.server.util.ConfigUtils;
 import org.apache.zookeeper.server.util.MessageTracker;
 import org.apache.zookeeper.server.util.ZxidUtils;
 import org.slf4j.Logger;
@@ -65,8 +66,10 @@ public class LearnerHandler extends ZooKeeperThread {
 
     private static final Logger LOG = 
LoggerFactory.getLogger(LearnerHandler.class);
 
-    public static final String LEADER_CLOSE_SOCKET_ASYNC = 
"leader.closeSocketAsync";
-    public static final boolean closeSocketAsync = 
Boolean.parseBoolean(System.getProperty(LEADER_CLOSE_SOCKET_ASYNC, "false"));
+    public static final String LEADER_CLOSE_SOCKET_ASYNC = 
"zookeeper.leader.closeSocketAsync";
+
+    public static final boolean closeSocketAsync = Boolean
+        
.parseBoolean(ConfigUtils.getPropertyBackwardCompatibleWay(LEADER_CLOSE_SOCKET_ASYNC));
 
     static {
         LOG.info("{} = {}", LEADER_CLOSE_SOCKET_ASYNC, closeSocketAsync);
diff --git 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/ConfigUtils.java
 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/ConfigUtils.java
index 508dc11..d6f7572 100644
--- 
a/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/ConfigUtils.java
+++ 
b/zookeeper-server/src/main/java/org/apache/zookeeper/server/util/ConfigUtils.java
@@ -95,4 +95,29 @@ public class ConfigUtils {
         }
     }
 
+    /**
+     * Some old configuration properties are not configurable in zookeeper 
configuration file
+     * zoo.cfg. To make these properties configurable in zoo.cfg old 
properties are prepended
+     * with zookeeper. For example prop.x.y.z changed to zookeeper.prop.x.y.z. 
But for backward
+     * compatibility both prop.x.y.z and zookeeper.prop.x.y.z should be 
supported.
+     * This method first gets value from new property, if first property is 
not configured
+     * then gets value from old property
+     *
+     * @param newPropertyKey new property key which starts with zookeeper.
+     * @return either new or old system property value. Null if none of the 
properties are set.
+     */
+    public static String getPropertyBackwardCompatibleWay(String 
newPropertyKey) {
+        String newKeyValue = System.getProperty(newPropertyKey);
+        if (newKeyValue != null) {
+            return newKeyValue.trim();
+        }
+        String oldPropertyKey = newPropertyKey.replace("zookeeper.", "");
+        String oldKeyValue = System.getProperty(oldPropertyKey);
+
+        if (oldKeyValue != null) {
+            return oldKeyValue.trim();
+        }
+        return null;
+    }
+
 }
diff --git 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/util/ConfigUtilsTest.java
 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/util/ConfigUtilsTest.java
index 4b293a0..e70c45c 100644
--- 
a/zookeeper-server/src/test/java/org/apache/zookeeper/server/util/ConfigUtilsTest.java
+++ 
b/zookeeper-server/src/test/java/org/apache/zookeeper/server/util/ConfigUtilsTest.java
@@ -19,6 +19,7 @@
 package org.apache.zookeeper.server.util;
 
 import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertNull;
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import org.apache.zookeeper.server.quorum.QuorumPeerConfig.ConfigException;
 import org.junit.jupiter.api.Test;
@@ -74,4 +75,45 @@ public class ConfigUtilsTest {
         assertEquals(nsa.length, 1);
     }
 
+    @Test
+    public void testGetPropertyBackwardCompatibleWay() throws ConfigException {
+        String newProp = "zookeeper.prop.x.y.z";
+        String oldProp = "prop.x.y.z";
+
+        // Null as both properties are not set
+        String result = ConfigUtils.getPropertyBackwardCompatibleWay(newProp);
+        assertNull(result);
+
+        // Return old property value when only old property is set
+        String oldPropValue = "oldPropertyValue";
+        System.setProperty(oldProp, oldPropValue);
+        result = ConfigUtils.getPropertyBackwardCompatibleWay(newProp);
+        assertEquals(oldPropValue, result);
+
+        // Return new property value when both properties are set
+        String newPropValue = "newPropertyValue";
+        System.setProperty(newProp, newPropValue);
+        result = ConfigUtils.getPropertyBackwardCompatibleWay(newProp);
+        assertEquals(newPropValue, result);
+
+        // cleanUp
+        clearProp(newProp, oldProp);
+
+        // Return trimmed value
+        System.setProperty(oldProp, oldPropValue + "  ");
+        result = ConfigUtils.getPropertyBackwardCompatibleWay(newProp);
+        assertEquals(oldPropValue, result);
+
+        System.setProperty(newProp, "  " + newPropValue);
+        result = ConfigUtils.getPropertyBackwardCompatibleWay(newProp);
+        assertEquals(newPropValue, result);
+
+        // cleanUp
+        clearProp(newProp, oldProp);
+    }
+
+    private void clearProp(String newProp, String oldProp) {
+        System.clearProperty(newProp);
+        System.clearProperty(oldProp);
+    }
 }

Reply via email to