From 3815f534eca5c33a611d70a16fcee38715f4d6a5 Mon Sep 17 00:00:00 2001
From: Piotr Grabowski <piotr.grabowski@scylladb.com>
Date: Fri, 1 Mar 2024 16:45:04 +0100
Subject: [PATCH] cluster: improve logging of peers row validation

Before this change, when the driver received an invalid system.peers row
it would log a very general warning:

    Found an invalid row for peer (127.0.73.5). Ignoring host.

A system.peers row can be invalid for a multitude of reasons and that
warning message did not describe the specific reason for the failure.

Improve the warning message by adding a specific reason why the row
is considered invalid by the driver. The message now also includes
the host_id or the entire row (in case the driver received a row
without even the basic broadcast_rpc).

It might be a bit inelegant to introduce a side effect (logging) to
the _is_valid_peer static method, however the alternative solution seemed
even worse - adding that code to the already big
_refresh_node_list_and_token_map.

Fixes #303
---
 cassandra/cluster.py | 40 ++++++++++++++++++++++++++++++++++------
 1 file changed, 34 insertions(+), 6 deletions(-)

diff --git a/cassandra/cluster.py b/cassandra/cluster.py
index 19d87b2a58..77ae703597 100644
--- a/cassandra/cluster.py
+++ b/cassandra/cluster.py
@@ -3950,9 +3950,6 @@ def _refresh_node_list_and_token_map(self, connection, preloaded_results=None,
         should_rebuild_token_map = force_token_rebuild or self._cluster.metadata.partitioner is None
         for row in peers_result:
             if not self._is_valid_peer(row):
-                log.warning(
-                    "Found an invalid row for peer (%s). Ignoring host." %
-                    _NodeInfo.get_broadcast_rpc_address(row))
                 continue
 
             endpoint = self._cluster.endpoint_factory.create(row)
@@ -4019,9 +4016,40 @@ def _refresh_node_list_and_token_map(self, connection, preloaded_results=None,
 
     @staticmethod
     def _is_valid_peer(row):
-        return bool(_NodeInfo.get_broadcast_rpc_address(row) and row.get("host_id") and
-                    row.get("data_center") and row.get("rack") and
-                    ('tokens' not in row or row.get('tokens')))
+        broadcast_rpc = _NodeInfo.get_broadcast_rpc_address(row)
+        host_id = row.get("host_id")
+
+        if not broadcast_rpc:
+            log.warning(
+                "Found an invalid row for peer - missing broadcast_rpc (full row: %s). Ignoring host." %
+                row)
+            return False
+
+        if not host_id:
+            log.warning(
+                "Found an invalid row for peer - missing host_id (broadcast_rpc: %s). Ignoring host." %
+                broadcast_rpc)
+            return False
+
+        if not row.get("data_center"):
+            log.warning(
+                "Found an invalid row for peer - missing data_center (broadcast_rpc: %s, host_id: %s). Ignoring host." %
+                (broadcast_rpc, host_id))
+            return False
+
+        if not row.get("rack"):
+            log.warning(
+                "Found an invalid row for peer - missing rack (broadcast_rpc: %s, host_id: %s). Ignoring host." %
+                (broadcast_rpc, host_id))
+            return False
+
+        if "tokens" in row and not row.get("tokens"):
+            log.warning(
+                "Found an invalid row for peer - tokens is None (broadcast_rpc: %s, host_id: %s). Ignoring host." %
+                (broadcast_rpc, host_id))
+            return False
+
+        return True
 
     def _update_location_info(self, host, datacenter, rack):
         if host.datacenter == datacenter and host.rack == rack: