Repository: hazelcast/hazelcast-python-client

Issue:
[TRACKING ISSUE] Increase default cluster timeout to 2 minutes
The tracking issue for the Java side PR.

**Increase default cluster timeout to 2 minutes**

See https://github.com/hazelcast/hazelcast/pull/18019 for details.

---

The old default tiemout was 20 seconds.
The change is done for environments like Kubernetes
where an Ip resolution/system restart could take
around a minute. We don't want a client to shutdown
in such cases, and we also want this to be out-of-the-box
behaviour.

Related config API

clientConfig.getConnectionStrategyConfig()
.getConnectionRetryConfig().getClusterConnectTimeoutMillis();

fixes https://github.com/hazelcast/hazelcast/issues/17904


Discussion:
mdumandag: Thanks for the review @yuce 

Patch:
diff --git a/docs/client_connection_strategy.rst b/docs/client_connection_strategy.rst
index 1365a66..e268cc7 100644
--- a/docs/client_connection_strategy.rst
+++ b/docs/client_connection_strategy.rst
@@ -46,10 +46,9 @@ starting and reconnecting modes.
 Configuring Client Connection Retry
 -----------------------------------
 
-When the client is disconnected from the cluster, it searches for new
-connections to reconnect. You can configure the frequency of the
-reconnection attempts and client shutdown behavior using the argumentes
-below.
+The client searches for new connections when it is trying to connect
+to the cluster. Both the frequency of connection attempts and the
+client shutdown behavior can be configured using the arguments below.
 
 .. code:: python
 
@@ -69,13 +68,13 @@ The following are configuration element descriptions:
 - ``retry_max_backoff``: Specifies the upper limit for the backoff in
   seconds. Its default value is ``30``. It must be non-negative.
 - ``retry_multiplier``: Factor to multiply the backoff after a failed
-  retry. Its default value is ``1``. It must be greater than or equal
+  retry. Its default value is ``1.05``. It must be greater than or equal
   to ``1``.
 - ``retry_jitter``: Specifies by how much to randomize backoffs. Its
   default value is ``0``. It must be in range ``0`` to ``1``.
 - ``cluster_connect_timeout``: Timeout value in seconds for the client
-  to give up to connect to the current cluster. Its default value is
-  ``120``.
+  to give up connecting to the cluster. Its default value is
+  ``-1``. The client will continuously try to connect by default.
 
 A pseudo-code is as follows:
 
@@ -86,6 +85,7 @@ A pseudo-code is as follows:
     while (try_connect(connection_timeout)) != SUCCESS) {
         if (get_current_time() - begin_time >= CLUSTER_CONNECT_TIMEOUT) {
             // Give up to connecting to the current cluster and switch to another if exists.
+            // CLUSTER_CONNECT_TIMEOUT is infinite by default.
         }
         sleep(current_backoff + uniform_random(-JITTER * current_backoff, JITTER * current_backoff))
         current_backoff = min(current_backoff * MULTIPLIER, MAX_BACKOFF)
diff --git a/hazelcast/client.py b/hazelcast/client.py
index 0306870..d41ecfb 100644
--- a/hazelcast/client.py
+++ b/hazelcast/client.py
@@ -1,7 +1,7 @@
 import logging
+import sys
 import threading
 
-from hazelcast import six
 from hazelcast.cluster import ClusterService, _InternalClusterService
 from hazelcast.config import _Config
 from hazelcast.connection import ConnectionManager, DefaultAddressProvider
@@ -121,10 +121,11 @@ class HazelcastClient(object):
             in range ``[0.0, 1.0]``. By default, set to ``0.0`` (no randomization).
         retry_multiplier (float): The factor with which to multiply backoff after a
             failed retry. Must be greater than or equal to ``1``. By default,
-            set to ``1.0``.
+            set to ``1.05``.
         cluster_connect_timeout (float): Timeout value in seconds for the client to
-            give up a connection attempt to the cluster. Must be non-negative.
-            By default, set to `120.0`.
+            give up connecting to the cluster. Must be non-negative or
+            equal to `-1`. By default, set to `-1`. `-1` means that the client
+            will not stop trying to the target cluster. (infinite timeout)
         portable_version (int): Default value for the portable version if the
             class does not have the :func:`get_portable_version` method. Portable
             versions are used to differentiate two versions of the
@@ -677,7 +678,7 @@ class HazelcastClient(object):
     @staticmethod
     def _get_connection_timeout(config):
         timeout = config.connection_timeout
-        return six.MAXSIZE if timeout == 0 else timeout
+        return sys.maxsize if timeout == 0 else timeout
 
     @staticmethod
     def _init_load_balancer(config):
diff --git a/hazelcast/config.py b/hazelcast/config.py
index 464fc73..bc02f31 100644
--- a/hazelcast/config.py
+++ b/hazelcast/config.py
@@ -459,6 +459,22 @@ class IndexUtil(object):
             raise ValueError("Unsupported index type %s" % index_type)
 
 
+_DEFAULT_CLUSTER_NAME = "dev"
+_DEFAULT_CONNECTION_TIMEOUT = 5.0
+_DEFAULT_RETRY_INITIAL_BACKOFF = 1.0
+_DEFAULT_RETRY_MAX_BACKOFF = 30.0
+_DEFAULT_RETRY_JITTER = 0.0
+_DEFAULT_RETRY_MULTIPLIER = 1.05
+_DEFAULT_CLUSTER_CONNECT_TIMEOUT = -1
+_DEFAULT_PORTABLE_VERSION = 0
+_DEFAULT_HEARTBEAT_INTERVAL = 5.0
+_DEFAULT_HEARTBEAT_TIMEOUT = 60.0
+_DEFAULT_INVOCATION_TIMEOUT = 120.0
+_DEFAULT_INVOCATION_RETRY_PAUSE = 1.0
+_DEFAULT_STATISTICS_PERIOD = 3.0
+_DEFAULT_OPERATION_BACKUP_TIMEOUT = 5.0
+
+
 class _Config(object):
     __slots__ = (
         "_cluster_members",
@@ -512,9 +528,9 @@ class _Config(object):
 
     def __init__(self):
         self._cluster_members = []
-        self._cluster_name = "dev"
+        self._cluster_name = _DEFAULT_CLUSTER_NAME
         self._client_name = None
-        self._connection_timeout = 5.0
+        self._connection_timeout = _DEFAULT_CONNECTION_TIMEOUT
         self._socket_options = []
         self._redo_operation = False
         self._smart_routing = True
@@ -528,12 +544,12 @@ class _Config(object):
         self._cloud_discovery_token = None
         self._async_start = False
         self._reconnect_mode = ReconnectMode.ON
-        self._retry_initial_backoff = 1.0
-        self._retry_max_backoff = 30.0
-        self._retry_jitter = 0.0
-        self._retry_multiplier = 1.0
-        self._cluster_connect_timeout = 120.0
-        self._portable_version = 0
+        self._retry_initial_backoff = _DEFAULT_RETRY_INITIAL_BACKOFF
+        self._retry_max_backoff = _DEFAULT_RETRY_MAX_BACKOFF
+        self._retry_jitter = _DEFAULT_RETRY_JITTER
+        self._retry_multiplier = _DEFAULT_RETRY_MULTIPLIER
+        self._cluster_connect_timeout = _DEFAULT_CLUSTER_CONNECT_TIMEOUT
+        self._portable_version = _DEFAULT_PORTABLE_VERSION
         self._data_serializable_factories = {}
         self._portable_factories = {}
         self._class_definitions = []
@@ -548,15 +564,15 @@ class _Config(object):
         self._lifecycle_listeners = []
         self._flake_id_generators = {}
         self._labels = []
-        self._heartbeat_interval = 5.0
-        self._heartbeat_timeout = 60.0
-        self._invocation_timeout = 120.0
-        self._invocation_retry_pause = 1.0
+        self._heartbeat_interval = _DEFAULT_HEARTBEAT_INTERVAL
+        self._heartbeat_timeout = _DEFAULT_HEARTBEAT_TIMEOUT
+        self._invocation_timeout = _DEFAULT_INVOCATION_TIMEOUT
+        self._invocation_retry_pause = _DEFAULT_INVOCATION_RETRY_PAUSE
         self._statistics_enabled = False
-        self._statistics_period = 3.0
+        self._statistics_period = _DEFAULT_STATISTICS_PERIOD
         self._shuffle_member_list = True
         self._backup_ack_to_client_enabled = True
-        self._operation_backup_timeout = 5.0
+        self._operation_backup_timeout = _DEFAULT_OPERATION_BACKUP_TIMEOUT
         self._fail_on_indeterminate_operation_state = False
 
     @property
@@ -812,8 +828,11 @@ class _Config(object):
     @cluster_connect_timeout.setter
     def cluster_connect_timeout(self, value):
         if isinstance(value, number_types):
-            if value < 0:
-                raise ValueError("cluster_connect_timeout must be non-negative")
+            if value < 0 and value != _DEFAULT_CLUSTER_CONNECT_TIMEOUT:
+                raise ValueError(
+                    "cluster_connect_timeout must be non-negative or equal to %s"
+                    % _DEFAULT_CLUSTER_CONNECT_TIMEOUT
+                )
             self._cluster_connect_timeout = value
         else:
             raise TypeError("cluster_connect_timeout must be a number")
diff --git a/hazelcast/connection.py b/hazelcast/connection.py
index 44053aa..841d9db 100644
--- a/hazelcast/connection.py
+++ b/hazelcast/connection.py
@@ -56,7 +56,7 @@ class _WaitStrategy(object):
         time_passed = time.time() - self._cluster_connect_attempt_begin
         if time_passed > self._cluster_connect_timeout:
             _logger.warning(
-                "Unable to get live cluster connection, cluster connect timeout (%d) is reached. "
+                "Unable to get live cluster connection, cluster connect timeout (%ds) is reached. "
                 "Attempt %d.",
                 self._cluster_connect_timeout,
                 self._attempt,
@@ -69,7 +69,7 @@ class _WaitStrategy(object):
         )
         sleep_time = min(sleep_time, self._cluster_connect_timeout - time_passed)
         _logger.warning(
-            "Unable to get live cluster connection, retry in %ds, attempt: %d, "
+            "Unable to get live cluster connection, retry in %.2fs, attempt: %d, "
             "cluster connect timeout: %ds, max backoff: %ds",
             sleep_time,
             self._attempt,
@@ -279,11 +279,18 @@ class ConnectionManager(object):
             self._start_connect_to_cluster_thread()
 
     def _init_wait_strategy(self, config):
+        cluster_connect_timeout = config.cluster_connect_timeout
+        if cluster_connect_timeout == -1:
+            # If the no timeout is specified by the
+            # user, or set to -1 explicitly, set
+            # the timeout to infinite.
+            cluster_connect_timeout = sys.maxsize
+
         return _WaitStrategy(
             config.retry_initial_backoff,
             config.retry_max_backoff,
             config.retry_multiplier,
-            config.cluster_connect_timeout,
+            cluster_connect_timeout,
             config.retry_jitter,
         )
 
diff --git a/hazelcast/cp.py b/hazelcast/cp.py
index e7541ae..b131c7b 100644
--- a/hazelcast/cp.py
+++ b/hazelcast/cp.py
@@ -1,7 +1,7 @@
+import sys
 import time
 from threading import RLock, Lock
 
-from hazelcast import six
 from hazelcast.errors import (
     SessionExpiredError,
     CPGroupDestroyedError,
@@ -277,7 +277,7 @@ class _SessionState(object):
     def _is_expired(self, timestamp):
         expiration_time = self.creation_time + self.ttl
         if expiration_time < 0:
-            expiration_time = six.MAXSIZE
+            expiration_time = sys.maxsize
         return timestamp > expiration_time
 
     def __eq__(self, other):
diff --git a/hazelcast/listener.py b/hazelcast/listener.py
index 52b30be..ffea93e 100644
--- a/hazelcast/listener.py
+++ b/hazelcast/listener.py
@@ -1,4 +1,5 @@
 import logging
+import sys
 import threading
 from uuid import uuid4
 
@@ -100,7 +101,7 @@ class ListenerService(object):
                     continue
 
                 invocation = Invocation(
-                    deregister_request, connection=connection, timeout=six.MAXSIZE, urgent=True
+                    deregister_request, connection=connection, timeout=sys.maxsize, urgent=True
                 )
                 self._invocation_service.invoke(invocation)
 


Test patch:
diff --git a/tests/config_test.py b/tests/config_test.py
index 722fdef..b313546 100644
--- a/tests/config_test.py
+++ b/tests/config_test.py
@@ -293,7 +293,7 @@ class ConfigTest(unittest.TestCase):
 
     def test_retry_multiplier(self):
         config = self.config
-        self.assertEqual(1, config.retry_multiplier)
+        self.assertEqual(1.05, config.retry_multiplier)
 
         with self.assertRaises(ValueError):
             config.retry_multiplier = 0.5
@@ -306,10 +306,13 @@ class ConfigTest(unittest.TestCase):
 
     def test_cluster_connect_timeout(self):
         config = self.config
-        self.assertEqual(120, config.cluster_connect_timeout)
+        self.assertEqual(-1, config.cluster_connect_timeout)
+
+        config.cluster_connect_timeout = -1
+        self.assertEqual(-1, config.cluster_connect_timeout)
 
         with self.assertRaises(ValueError):
-            config.cluster_connect_timeout = -1
+            config.cluster_connect_timeout = -2
 
         with self.assertRaises(TypeError):
             config.cluster_connect_timeout = ""
diff --git a/tests/connection_strategy_test.py b/tests/connection_strategy_test.py
index 65660b7..a040662 100644
--- a/tests/connection_strategy_test.py
+++ b/tests/connection_strategy_test.py
@@ -87,7 +87,6 @@ class ConnectionStrategyTest(HazelcastTestCase):
             cluster_members=["localhost:5701"],
             cluster_name=self.cluster.id,
             reconnect_mode=ReconnectMode.OFF,
-            cluster_connect_timeout=six.MAXSIZE,
             lifecycle_listeners=[event_collector],
         )
         m = self.client.get_map(random_string()).blocking()
@@ -119,7 +118,6 @@ class ConnectionStrategyTest(HazelcastTestCase):
             cluster_members=["localhost:5701"],
             cluster_name=self.cluster.id,
             reconnect_mode=ReconnectMode.ASYNC,
-            cluster_connect_timeout=six.MAXSIZE,
             lifecycle_listeners=[disconnected_collector],
         )
         m = self.client.get_map(random_string()).blocking()