Populate the DRIVER_CONFIG report - phase 2

CHANGELOG.rst

@@ -14,12 +14,43 @@ Features
   ``Cluster(driver_config_reporting_enabled=False)``; ``SESSION_ID`` is unaffected by
   that setting. Reporting is best effort and never prevents a connection from being
   established.
+* ``DRIVER_CONFIG`` now describes the configuration itself rather than only the schema
+  version it follows (DRIVER-379). The report covers connection settings (timeouts,
+  request capacity, shard awareness, socket options, reconnection policy, TLS hostname
+  verification), the driver's own control-plane query timeouts, and the query defaults
+  and policies a statement gets when it overrides none of them. It follows the JSON
+  schema shared with the other ScyllaDB drivers, so the same document describes a
+  client whichever driver wrote it. Custom policies are reported by type name only and
+  never by their attributes, so a policy holding a credential does not leak it into the
+  clients table.
+* ``Cluster.sockopts`` is now materialized at construction, so a one-shot iterable is
+  applied to every connection the cluster opens rather than only to the first one.
 * Negotiate and implement the ``SCYLLA_USE_METADATA_ID`` protocol extension: prepared
   statements skip re-sending result metadata on EXECUTE, and the driver automatically
   refreshes cached metadata when the server detects a schema change (DRIVER-153)
 
 Others
 ------
+* ``DCAwareRoundRobinPolicy.local_dc`` is now read-only. It is set by the constructor,
+  and filled in by the policy itself when the constructor was given none, from the first
+  host to come up. Assigning it afterwards was indistinguishable from that inference,
+  and the two mean different things: a datacenter the application chose against one the
+  driver guessed. Code that assigned it should pass ``local_dc`` to the constructor
+  instead.
+* ``Connection.max_request_id`` and ``Connection.orphaned_threshold`` now follow the
+  ``max_in_flight`` actually in force. Both were computed in the class body, which runs
+  once, so a subclass that set its own ``max_in_flight`` inherited values derived from the
+  base class -- leaving, for example, a ``max_in_flight`` of 256 with a threshold of
+  24576, which a connection holding at most 256 orphaned stream ids can never reach, so
+  orphan-based connection replacement never happened for such a subclass. Each connection
+  now derives both in ``__init__`` from the limit in force when it is built, which
+  overrides a value a subclass sets in its class body. ``orphaned_threshold`` is also
+  capped at three quarters of the CQL stream id range, as ``max_request_id`` already was:
+  a ``max_in_flight`` raised past that range left the threshold above the number of stream
+  ids a connection can hold at all, which is the same bug in the other direction. The two
+  new static methods ``Connection.max_request_id_for()`` and
+  ``Connection.orphaned_threshold_for()`` expose the derivation, so that both limits can
+  be read for a given ``max_in_flight`` before any connection exists.
 * The ``STARTUP`` options that describe the driver itself are no longer the
   application's to set. An ``ApplicationInfoBase.add_startup_options`` that sets
   ``DRIVER_NAME``, ``DRIVER_VERSION``, ``SESSION_ID`` or ``DRIVER_CONFIG`` now has that

cassandra/cluster.py

@@ -1468,7 +1468,17 @@ def __init__(self,
 
         self.ssl_options = ssl_options
         self.ssl_context = ssl_context
-        self.sockopts = sockopts
+        # Materialized once: these are applied to every socket the cluster opens
+        # and are read again to build the configuration report, so a one-shot
+        # iterable would leave whichever consumer ran second with nothing at all.
+        # Something that is not a sequence of options at all is kept as it was
+        # given, so that it still fails where it always did -- on the socket, at
+        # connect time -- rather than turning a constructor that used to build
+        # into one that raises.
+        try:
+            self.sockopts = list(sockopts) if sockopts is not None else None
+        except TypeError:
+            self.sockopts = sockopts
         self.cql_version = cql_version
         self.max_schema_agreement_wait = max_schema_agreement_wait
         self.control_connection_timeout = control_connection_timeout
@@ -1520,7 +1530,7 @@ def __init__(self,
         # Built whatever the flag says, so that the flag is the only thing that
         # decides whether a connection reports: see _make_connection_kwargs. The
         # reporter holds no state, so an unused one costs nothing.
-        self._driver_config_reporter = DriverConfigReporter()
+        self._driver_config_reporter = DriverConfigReporter(self)
 
         self.control_connection = ControlConnection(
             self, self.control_connection_timeout,

cassandra/connection.py

@@ -844,10 +844,50 @@ class Connection(object):
     # and the connection will be replaced
     orphaned_threshold_reached = False
 
+    # The CQL stream id space, which is all the protocol can address however high
+    # max_in_flight is set. Both limits below are capped to it.
+    _MAX_STREAM_IDS = 2 ** 15
+
     # If the number of orphaned streams reaches this threshold, this connection
     # will become marked and will be replaced with a new connection by the
-    # owning pool (currently, only HostConnection supports this)
-    orphaned_threshold = 3  * max_in_flight // 4
+    # owning pool (currently, only HostConnection supports this). The default
+    # for this class's max_in_flight; a connection derives its own in __init__.
+    orphaned_threshold = 3 * min(max_in_flight, _MAX_STREAM_IDS) // 4
+
+    @staticmethod
+    def max_request_id_for(max_in_flight):
+        """
+        The highest request id a connection with this limit will hand out.
+
+        Request ids run from zero to this inclusive, and borrow_connection
+        admits a request only while in_flight is below it. Capped at the CQL
+        stream id range, which is all the protocol can address however high
+        max_in_flight is set.
+        """
+        return min(max_in_flight, Connection._MAX_STREAM_IDS) - 1
+
+    @staticmethod
+    def orphaned_threshold_for(max_in_flight):
+        """
+        The orphaned stream count at which a connection with this limit is
+        marked for replacement.
+
+        Three quarters of the stream ids a connection can actually hold, which
+        is max_in_flight capped the way :meth:`max_request_id_for` caps it.
+        Taken off that capped pool rather than off max_in_flight itself: a
+        connection holds at most max_request_id + 1 ids, so a threshold above
+        that is one `len(orphaned_request_ids) >= orphaned_threshold` never
+        reaches, leaving orphan-based replacement dead for a max_in_flight
+        raised past the stream id range.
+        """
+        return 3 * min(max_in_flight, Connection._MAX_STREAM_IDS) // 4
+
+    # Both limits are derived, and both are asked for rather than stored on the
+    # class, because max_in_flight is tuned at runtime -- assigned on the class,
+    # or patched in a test -- and a value derived once does not follow it. A
+    # connection derives both in __init__ from the limit in force when it is
+    # built, and the configuration report, which has to describe them before any
+    # connection exists, asks with the class's current limit.
 
     is_defunct = False
     is_closed = False
@@ -944,7 +984,9 @@ def __init__(self, host='127.0.0.1', port=9042, authenticator=None,
         if not self.ssl_context and self.ssl_options:
             self.ssl_context = self._build_ssl_context_from_options()
 
-        self.max_request_id = min(self.max_in_flight - 1, (2 ** 15) - 1)
+        self.max_request_id = self.max_request_id_for(self.max_in_flight)
+        self.orphaned_threshold = self.orphaned_threshold_for(self.max_in_flight)
+
         # Don't fill the deque with 2**15 items right away. Start with some and add
         # more if needed.
         initial_size = min(300, self.max_in_flight)
@@ -1563,7 +1605,13 @@ def _handle_options_response(self, options_response):
         # only the control connection reports it. A reporter left as None means
         # the cluster has configuration reporting disabled.
         if self.is_control_connection and self._driver_config_reporter is not None:
-            self._driver_config_reporter.add_startup_options(options)
+            # Whether this is a ScyllaDB node is already known: the features
+            # above were parsed from the SUPPORTED response, and sharding info
+            # is what the driver itself keys ScyllaDB-only behaviour off (see
+            # ControlConnection._try_connect), so the report describes what the
+            # driver will actually do rather than only what it was configured to.
+            self._driver_config_reporter.add_startup_options(
+                options, is_scylla=self.features.sharding_info is not None)
 
         if self.cql_version:
             if self.cql_version not in supported_cql_versions: