You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@aurora.apache.org by se...@apache.org on 2017/10/15 16:00:27 UTC

aurora git commit: Use compatible Curator session and connection timeouts

Repository: aurora
Updated Branches:
  refs/heads/master 2aee90d0e -> c446504e6


Use compatible Curator session and connection timeouts

Curator will warn if used with a connection timeout that is lower than
the session timeout [1]. As it uses a default connection timeout of 15s
[2], this warning will be emitted using the Aurora default settings.

This patch remedies this issue in two ways:

* Making the Curator connection timeout configurable
* Bumping the session timeout to 15s. The current default of 4s is
  pretty small and could lead to unexpected failovers during long GC
  pauses. This is especially problematic as a failover in Aurora can
  be lengthy.

[1] https://github.com/apache/curator/blob/15eb063fa22569e797f850fb8d60a0949f52fbf5/curator-client/src/main/java/org/apache/curator/CuratorZookeeperClient.java#L118-L121
[2] https://github.com/apache/curator/blob/6ba4de36d4e8b2b65d45c005a6a92dd85c3c497f/curator-framework/src/main/java/org/apache/curator/framework/CuratorFrameworkFactory.java#L60-L61

Reviewed at https://reviews.apache.org/r/62835/


Project: http://git-wip-us.apache.org/repos/asf/aurora/repo
Commit: http://git-wip-us.apache.org/repos/asf/aurora/commit/c446504e
Tree: http://git-wip-us.apache.org/repos/asf/aurora/tree/c446504e
Diff: http://git-wip-us.apache.org/repos/asf/aurora/diff/c446504e

Branch: refs/heads/master
Commit: c446504e63b8a7bb03845c0d9eaefe072fe02e91
Parents: 2aee90d
Author: Stephan Erb <se...@apache.org>
Authored: Sun Oct 15 18:00:17 2017 +0200
Committer: Stephan Erb <se...@apache.org>
Committed: Sun Oct 15 18:00:17 2017 +0200

----------------------------------------------------------------------
 RELEASE-NOTES.md                                       |  2 ++
 .../apache/aurora/common/zookeeper/ZooKeeperUtils.java |  4 +++-
 .../discovery/CuratorServiceDiscoveryModule.java       |  1 +
 .../scheduler/discovery/FlaggedZooKeeperConfig.java    |  5 +++++
 .../aurora/scheduler/discovery/ZooKeeperConfig.java    | 13 +++++++++++++
 .../aurora/scheduler/config/CommandLineTest.java       |  2 ++
 .../discovery/AbstractDiscoveryModuleTest.java         |  1 +
 .../scheduler/discovery/ZooKeeperConfigTest.java       |  3 +++
 8 files changed, 30 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/RELEASE-NOTES.md
----------------------------------------------------------------------
diff --git a/RELEASE-NOTES.md b/RELEASE-NOTES.md
index c58e680..079f495 100644
--- a/RELEASE-NOTES.md
+++ b/RELEASE-NOTES.md
@@ -15,6 +15,8 @@
   `stop_timeout_in_secs` flag.
 - Added the `thrift_method_interceptor_modules` scheduler flag that lets cluster operators inject
   custom Thrift method interceptors.
+- Increase default ZooKeeper session timeout from 4 to 15 seconds.
+- Add option `-zk_connection_timeout` to control the connection timeout of ZooKeeper connections.
 
 ### Deprecations and removals
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java
----------------------------------------------------------------------
diff --git a/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java b/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java
index 2ada264..93ddd89 100644
--- a/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java
+++ b/commons/src/main/java/org/apache/aurora/common/zookeeper/ZooKeeperUtils.java
@@ -39,7 +39,9 @@ public final class ZooKeeperUtils {
   /**
    * An appropriate default session timeout for Twitter ZooKeeper clusters.
    */
-  public static final Amount<Integer,Time> DEFAULT_ZK_SESSION_TIMEOUT = Amount.of(4, Time.SECONDS);
+  public static final Amount<Integer,Time> DEFAULT_ZK_SESSION_TIMEOUT = Amount.of(15, Time.SECONDS);
+
+  public static final Amount<Integer,Time> DEFAULT_ZK_CONNECTION_TIMEOUT = Amount.of(10, Time.SECONDS);
 
   /**
    * The magic version number that allows any mutation to always succeed regardless of actual

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
index ea167a8..40cda8c 100644
--- a/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
+++ b/src/main/java/org/apache/aurora/scheduler/discovery/CuratorServiceDiscoveryModule.java
@@ -135,6 +135,7 @@ class CuratorServiceDiscoveryModule extends PrivateModule {
         .connectString(connectString)
         .canBeReadOnly(false) // We must be able to write to perform leader election.
         .sessionTimeoutMs(zooKeeperConfig.getSessionTimeout().as(Time.MILLISECONDS))
+        .connectionTimeoutMs(zooKeeperConfig.getConnectionTimeout().as(Time.MILLISECONDS))
         .retryPolicy(retryPolicy)
         .aclProvider(aclProvider);
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java b/src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
index c2e8ce2..1e7b9ce 100644
--- a/src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
+++ b/src/main/java/org/apache/aurora/scheduler/discovery/FlaggedZooKeeperConfig.java
@@ -29,6 +29,7 @@ import org.apache.aurora.common.zookeeper.Credentials;
 import org.apache.aurora.scheduler.config.types.TimeAmount;
 import org.apache.aurora.scheduler.config.validators.NotEmptyIterable;
 
+import static org.apache.aurora.scheduler.discovery.ZooKeeperConfig.DEFAULT_CONNECTION_TIMEOUT;
 import static org.apache.aurora.scheduler.discovery.ZooKeeperConfig.DEFAULT_SESSION_TIMEOUT;
 
 /**
@@ -67,6 +68,9 @@ public final class FlaggedZooKeeperConfig {
     @Parameter(names = "-zk_session_timeout", description = "The ZooKeeper session timeout.")
     public TimeAmount sessionTimeout = DEFAULT_SESSION_TIMEOUT;
 
+    @Parameter(names = "-zk_connection_timeout", description = "The ZooKeeper connection timeout.")
+    public TimeAmount connectionTimeout = DEFAULT_CONNECTION_TIMEOUT;
+
     @Parameter(names = "-zk_digest_credentials",
         description = "user:password to use when authenticating with ZooKeeper.")
     public String digestCredentials;
@@ -88,6 +92,7 @@ public final class FlaggedZooKeeperConfig {
         Optional.fromNullable(opts.chrootPath),
         opts.inProcess,
         Amount.of(opts.sessionTimeout.getValue().intValue(), opts.sessionTimeout.getUnit()),
+        Amount.of(opts.connectionTimeout.getValue().intValue(), opts.connectionTimeout.getUnit()),
         getCredentials(opts.digestCredentials));
   }
 

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java
----------------------------------------------------------------------
diff --git a/src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java b/src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java
index f6faca5..433ed31 100644
--- a/src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java
+++ b/src/main/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfig.java
@@ -37,6 +37,10 @@ public class ZooKeeperConfig {
       ZooKeeperUtils.DEFAULT_ZK_SESSION_TIMEOUT.getValue(),
         ZooKeeperUtils.DEFAULT_ZK_SESSION_TIMEOUT.getUnit());
 
+  public static final TimeAmount DEFAULT_CONNECTION_TIMEOUT = new TimeAmount(
+      ZooKeeperUtils.DEFAULT_ZK_CONNECTION_TIMEOUT.getValue(),
+        ZooKeeperUtils.DEFAULT_ZK_CONNECTION_TIMEOUT.getUnit());
+
   /**
    * Creates a new client configuration with defaults for the session timeout and credentials.
    *
@@ -51,6 +55,7 @@ public class ZooKeeperConfig {
         Optional.absent(), // chrootPath
         false,
         ZooKeeperUtils.DEFAULT_ZK_SESSION_TIMEOUT,
+        ZooKeeperUtils.DEFAULT_ZK_CONNECTION_TIMEOUT,
         Optional.absent()); // credentials
   }
 
@@ -58,6 +63,7 @@ public class ZooKeeperConfig {
   private final Iterable<InetSocketAddress> servers;
   private final boolean inProcess;
   private final Amount<Integer, Time> sessionTimeout;
+  private final Amount<Integer, Time> connectionTimeout;
   private final Optional<String> chrootPath;
   private final Optional<Credentials> credentials;
 
@@ -76,6 +82,7 @@ public class ZooKeeperConfig {
       Optional<String> chrootPath,
       boolean inProcess,
       Amount<Integer, Time> sessionTimeout,
+      Amount<Integer, Time> connectionTimeout,
       Optional<Credentials> credentials) {
 
     this.useCurator = useCurator;
@@ -83,6 +90,7 @@ public class ZooKeeperConfig {
     this.chrootPath = requireNonNull(chrootPath);
     this.inProcess = inProcess;
     this.sessionTimeout = requireNonNull(sessionTimeout);
+    this.connectionTimeout = requireNonNull(connectionTimeout);
     this.credentials = requireNonNull(credentials);
   }
 
@@ -100,6 +108,7 @@ public class ZooKeeperConfig {
         chrootPath,
         inProcess,
         sessionTimeout,
+        connectionTimeout,
         Optional.of(newCredentials));
   }
 
@@ -119,6 +128,10 @@ public class ZooKeeperConfig {
     return sessionTimeout;
   }
 
+  public Amount<Integer, Time> getConnectionTimeout() {
+    return connectionTimeout;
+  }
+
   Optional<String> getChrootPath() {
     return chrootPath;
   }

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java b/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
index 9b4f2ad..1e2e01d 100644
--- a/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/config/CommandLineTest.java
@@ -164,6 +164,7 @@ public class CommandLineTest {
     expected.zk.zkEndpoints = ImmutableList.of(InetSocketAddress.createUnresolved("testing", 42));
     expected.zk.chrootPath = "testing";
     expected.zk.sessionTimeout = TEST_TIME;
+    expected.zk.connectionTimeout = TEST_TIME;
     expected.zk.digestCredentials = "testing";
     expected.updater.enableAffinity = true;
     expected.updater.affinityExpiration = TEST_TIME;
@@ -315,6 +316,7 @@ public class CommandLineTest {
         "-zk_endpoints=testing:42",
         "-zk_chroot_path=testing",
         "-zk_session_timeout=42days",
+        "-zk_connection_timeout=42days",
         "-zk_digest_credentials=testing",
         "-enable_update_affinity=true",
         "-update_affinity_reservation_hold_time=42days",

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java
index 0f2121e..cec54e5 100644
--- a/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/discovery/AbstractDiscoveryModuleTest.java
@@ -47,6 +47,7 @@ abstract class AbstractDiscoveryModuleTest extends TearDownTestCase {
             Optional.of("/chroot"),
             false, // inProcess
             Amount.of(1, Time.DAYS),
+            Amount.of(1, Time.DAYS),
             Optional.of(Credentials.digestCredentials("test", "user")));
 
     Injector injector =

http://git-wip-us.apache.org/repos/asf/aurora/blob/c446504e/src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java
----------------------------------------------------------------------
diff --git a/src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java b/src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java
index a065505..baee123 100644
--- a/src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java
+++ b/src/test/java/org/apache/aurora/scheduler/discovery/ZooKeeperConfigTest.java
@@ -42,6 +42,7 @@ public class ZooKeeperConfigTest {
         Optional.absent(),
         false,
         Amount.of(1, Time.DAYS),
+        Amount.of(1, Time.DAYS),
         Optional.absent());
   }
 
@@ -54,6 +55,7 @@ public class ZooKeeperConfigTest {
             Optional.absent(),
             false,
             Amount.of(1, Time.HOURS),
+            Amount.of(1, Time.HOURS),
             Optional.absent()); // credentials
     assertFalse(config.getCredentials().isPresent());
 
@@ -77,6 +79,7 @@ public class ZooKeeperConfigTest {
     assertFalse(config.getChrootPath().isPresent());
     assertFalse(config.isInProcess());
     assertEquals(ZooKeeperUtils.DEFAULT_ZK_SESSION_TIMEOUT, config.getSessionTimeout());
+    assertEquals(ZooKeeperUtils.DEFAULT_ZK_CONNECTION_TIMEOUT, config.getConnectionTimeout());
     assertFalse(config.getCredentials().isPresent());
   }
 }