Set enable_cleanup_closed by default (#1469)

Co-authored-by: Rick Boyd <[email protected]>

The timings changes are unrelated to the `enable_cleanup_closed` change.

docs/command_line_reference.rst

@@ -181,7 +181,7 @@ Selects the track repository that Rally should use to resolve tracks. By default
 ``track-revision``
 ~~~~~~~~~~~~~~~~~~
 
-Selects a specific revision in the track repository. By default, Rally will choose the most appropriate branch on its own but in some cases it is necessary to specify a certain commit. This is mostly needed when testing whether a change in performance has occurred due to a change in the workload. 
+Selects a specific revision in the track repository. By default, Rally will choose the most appropriate branch on its own but in some cases it is necessary to specify a certain commit. This is mostly needed when testing whether a change in performance has occurred due to a change in the workload.
 
 ``track``
 ~~~~~~~~~
@@ -656,7 +656,7 @@ Default value: ``timeout:60`` (applies any time ``timeout`` is not specified)
 Rally recognizes the following client options in addition:
 
 * ``max_connections``: By default, Rally will choose the maximum allowed number of connections automatically (equal to the number of simulated clients but at least 256 connections). With this property it is possible to override that logic but a minimum of 256 is enforced internally.
-* ``enable_cleanup_closed`` (default: ``false``): In some cases, SSL connections might not be properly closed and the number of open connections increases as a result. When this client option is set to ``true``, the Elasticsearch client will check and forcefully close these connections.
+* ``enable_cleanup_closed`` (default: ``true``): In some cases, `Elasticsearch does not properly close SSL connections <https://github.com/elastic/elasticsearch/issues/76642>`_ and the number of open connections increases as a result. When this client option is set to ``true``, the Elasticsearch client will check and forcefully close these connections.
 * ``static_responses``: The path to a JSON file containing path patterns and the corresponding responses. When this value is set to ``true``, Rally will not send requests to Elasticsearch but return static responses as specified by the file. This is useful to diagnose performance issues in Rally itself. See below for a specific example.
 
 **Examples**

esrally/async_connection.py

@@ -206,7 +206,7 @@ def __init__(
         )
 
         self._trace_configs = [trace_config] if trace_config else None
-        self._enable_cleanup_closed = kwargs.get("enable_cleanup_closed", False)
+        self._enable_cleanup_closed = kwargs.get("enable_cleanup_closed", True)
 
         static_responses = kwargs.get("static_responses")
         self.use_static_responses = static_responses is not None

tests/client_test.py

@@ -28,6 +28,7 @@
 import urllib3.exceptions
 
 from esrally import client, doc_link, exceptions
+from esrally.async_connection import AIOHttpConnection
 from esrally.utils import console
 from tests import run_async
 
@@ -349,3 +350,15 @@ def test_ssl_error(self, es):
         )
         with pytest.raises(exceptions.SystemSetupError, match="Could not connect to cluster via https. Is this an https endpoint?"):
             client.wait_for_rest_layer(es, max_attempts=3)
+
+
+class TestAsyncConnection:
+    @run_async
+    async def test_enable_cleanup_close(self):
+        connection = AIOHttpConnection()
+        # pylint: disable=protected-access
+        assert connection._enable_cleanup_closed is True
+
+        connection = AIOHttpConnection(enable_cleanup_closed=False)
+        # pylint: disable=protected-access
+        assert connection._enable_cleanup_closed is False

tests/driver/runner_test.py

@@ -5795,13 +5795,13 @@ async def test_adds_request_timings(self):
         assert len(timings) == 3
 
         assert timings[0]["operation"] == "initial-call"
-        assert math.isclose(timings[0]["service_time"], 0.1, rel_tol=0.05)
+        assert timings[0]["service_time"] == pytest.approx(0.1, abs=0.1)
 
         assert timings[1]["operation"] == "stream-a"
-        assert math.isclose(timings[1]["service_time"], 0.2, rel_tol=0.05)
+        assert timings[1]["service_time"] == pytest.approx(0.2, abs=0.1)
 
         assert timings[2]["operation"] == "stream-b"
-        assert math.isclose(timings[2]["service_time"], 0.1, rel_tol=0.05)
+        assert timings[2]["service_time"] == pytest.approx(0.1, abs=0.1)
 
         # common properties
         for timing in timings: