Don't create Host instances with random host_id #623

sylwiaszunejko · 2025-12-18T13:21:21Z

This PR fixes inefficiencies in the host initialization mechanism when bootstrapping a cluster.

Previously, the driver created Host instances with connections from the contact points provided in the cluster configuration using random host IDs. After establishing the control connection and reading from system.peers, these initial Host instances were discarded and replaced with new ones created using the correct host metadata. This approach resulted in unnecessary creation and teardown of multiple connections.

Changes

The control connection is now initialized only using the endpoints specified in the cluster configuration.
After a successful control connection is established, the driver reads from system.local and system.peers.
Based on this metadata, Host instances are created with the correct host_id values.
Connections are then initialized directly on these properly constructed Host instances.

Fixes: #619

sylwiaszunejko · 2025-12-18T13:21:56Z

Some tests are still failing, but I wanted to ask if the direction is good @dkropachev

sylwiaszunejko · 2025-12-18T14:57:06Z

@Lorak-mmk maybe you know, why this test assumes that the new_host should be different?

def test_get_control_connection_host(self):
        """
        Test to validate Cluster.get_control_connection_host() metadata

        @since 3.5.0
        @jira_ticket PYTHON-583
        @expected_result the control connection metadata should accurately reflect cluster state.

        @test_category metadata
        """

        host = self.cluster.get_control_connection_host()
        assert host == None

        self.session = self.cluster.connect()
        cc_host = self.cluster.control_connection._connection.host

        host = self.cluster.get_control_connection_host()
        assert host.address == cc_host
        assert host.is_up == True

        # reconnect and make sure that the new host is reflected correctly
        self.cluster.control_connection._reconnect()
        new_host = self.cluster.get_control_connection_host()
        assert host != new_host

Lorak-mmk · 2025-12-18T15:12:17Z

I have no idea.
In Rust Driver we have logic that if CC breaks then we try to connect it to all other hosts (because the one it was connected to is presumed non-working for now).
I see no such logic in Python Driver. This part was added in commit 2796ee5:

Was this test passing until now and non-flaky? If so, then perhaps there is such logic somewhere.

Lorak-mmk · 2025-12-18T15:13:04Z

Now that I think of it: I see that driver uses LBP to decide order of hosts to connect. See _connect_host_in_lbp and _reconnect_internal.
LBP uses by default is Round Robin, so on reconnect it will start from a different host than at the beginning, right? It would explain why each CC reconnect should land at different host in healthy cluster.

sylwiaszunejko · 2025-12-18T15:21:00Z

Now that I think of it: I see that driver uses LBP to decide order of hosts to connect. See _connect_host_in_lbp and _reconnect_internal. LBP uses by default is Round Robin, so on reconnect it will start from a different host than at the beginning, right? It would explain why each CC reconnect should land at different host in healthy cluster.

Makes sense, second question: in this test:

def test_profile_lb_swap(self):
        """
        Tests that profile load balancing policies are not shared

        Creates two LBP, runs a few queries, and validates that each LBP is execised
        seperately between EP's

        @since 3.5
        @jira_ticket PYTHON-569
        @expected_result LBP should not be shared.

        @test_category config_profiles
        """
        query = "select release_version from system.local where key='local'"
        rr1 = ExecutionProfile(load_balancing_policy=RoundRobinPolicy())
        rr2 = ExecutionProfile(load_balancing_policy=RoundRobinPolicy())
        exec_profiles = {'rr1': rr1, 'rr2': rr2}
        with TestCluster(execution_profiles=exec_profiles) as cluster:
            session = cluster.connect(wait_for_all_pools=True)

            # default is DCA RR for all hosts
            expected_hosts = set(cluster.metadata.all_hosts())
            rr1_queried_hosts = set()
            rr2_queried_hosts = set()

            rs = session.execute(query, execution_profile='rr1')
            rr1_queried_hosts.add(rs.response_future._current_host)
            rs = session.execute(query, execution_profile='rr2')
            rr2_queried_hosts.add(rs.response_future._current_host)

            assert rr2_queried_hosts == rr1_queried_hosts

in this tests it is assumed that both queries should use the same host, as they use different instances of RoundRobinPolicy and they start from the same host? But how this can be true if the position when we start is randomized here: https://github.com/scylladb/python-driver/blob/master/cassandra/policies.py#L182

Lorak-mmk · 2025-12-18T15:38:37Z

No idea. Perhaps populate is not called for those policies for some reason, and they are populated using on_up/down etc?
Try to print a log / stacktrace in populate and run this test.

cassandra/cluster.py

cassandra/policies.py

cassandra/cluster.py

sylwiaszunejko · 2025-12-19T14:34:02Z

Now that I think of it: I see that driver uses LBP to decide order of hosts to connect. See _connect_host_in_lbp and _reconnect_internal. LBP uses by default is Round Robin, so on reconnect it will start from a different host than at the beginning, right? It would explain why each CC reconnect should land at different host in healthy cluster.

Makes sense, second question: in this test:
def test_profile_lb_swap(self):
        """
        Tests that profile load balancing policies are not shared

        Creates two LBP, runs a few queries, and validates that each LBP is execised
        seperately between EP's

        @since 3.5
        @jira_ticket PYTHON-569
        @expected_result LBP should not be shared.

        @test_category config_profiles
        """
        query = "select release_version from system.local where key='local'"
        rr1 = ExecutionProfile(load_balancing_policy=RoundRobinPolicy())
        rr2 = ExecutionProfile(load_balancing_policy=RoundRobinPolicy())
        exec_profiles = {'rr1': rr1, 'rr2': rr2}
        with TestCluster(execution_profiles=exec_profiles) as cluster:
            session = cluster.connect(wait_for_all_pools=True)

            # default is DCA RR for all hosts
            expected_hosts = set(cluster.metadata.all_hosts())
            rr1_queried_hosts = set()
            rr2_queried_hosts = set()

            rs = session.execute(query, execution_profile='rr1')
            rr1_queried_hosts.add(rs.response_future._current_host)
            rs = session.execute(query, execution_profile='rr2')
            rr2_queried_hosts.add(rs.response_future._current_host)

            assert rr2_queried_hosts == rr1_queried_hosts
in this tests it is assumed that both queries should use the same host, as they use different instances of RoundRobinPolicy and they start from the same host? But how this can be true if the position when we start is randomized here: https://github.com/scylladb/python-driver/blob/master/cassandra/policies.py#L182

This test was working because populate was called before cc was created, so we only knew about contact points provided in cluster config (so only one host) I believe current approach (calling populate on lbp after creating cc so we can update lbp with all known hosts) is much better so we should remove this test @Lorak-mmk WDYT?

Lorak-mmk · 2025-12-19T15:13:24Z

In the previous approach (calling populate with one host) were the on_add calls correct (so one call for each host, besides CC host)?
If so, then both versions are correct. I think we could then switch to proposed version.

Lorak-mmk · 2025-12-19T15:13:50Z

You could then adjust the test, not remove it.

sylwiaszunejko · 2025-12-19T15:34:14Z

In the previous approach (calling populate with one host) were the on_add calls correct (so one call for each host, besides CC host)? If so, then both versions are correct. I think we could then switch to proposed version.

on_add is called properly, but if there is only one host during populate the starting position for RoundRobinPolicy is always the same even if some hosts are added later:

if len(hosts) > 1:
            self._position = randint(0, len(hosts) - 1)

cassandra/cluster.py

Lorak-mmk · 2025-12-29T12:40:59Z

Please let me review before merging,

tests/integration/standard/test_query.py

tests/integration/standard/test_policies.py

Lorak-mmk · 2025-12-29T13:07:11Z

tests/integration/standard/test_metrics.py

+            with pytest.raises((WriteTimeout, Unavailable)):
                self.session.execute(query, timeout=None)
        finally:
            get_node(1).resume()

        # Change the scales stats_name of the cluster2
        cluster2.metrics.set_stats_name('cluster2-metrics')

        stats_cluster1 = self.cluster.metrics.get_stats()
        stats_cluster2 = cluster2.metrics.get_stats()

        # Test direct access to stats
-        assert 1 == self.cluster.metrics.stats.write_timeouts
+        assert (1 == self.cluster.metrics.stats.write_timeouts or 1 == self.cluster.metrics.stats.unavailables)
        assert 0 == cluster2.metrics.stats.write_timeouts


Why did the exception thrown change?

It is an expected error in given scenario.
Only difference between WriteTimeout and Unavailable is WriteTimeout happens when coordinator scheduled a query not knowing that node is down and not getting response back, while Unavailable is when coordinator knows that node is down and do not even try.

So, reason is why we start getting Unavailable is that a delay between get_node(1).pause() and self.session.execute(query, timeout=None) is increased, which is most-likely has something to do with absence of dropping node pool for fake node records driver used to create before.

I would say it is safe to just add Unavailable to expected exceptions and move on.

tests/integration/standard/test_control_connection.py

tests/integration/standard/test_cluster.py

cassandra/policies.py

cassandra/metadata.py

cassandra/cluster.py

sylwiaszunejko · 2025-12-30T10:40:07Z

@Lorak-mmk I haven't yet figured out why in test_metrics_per_cluster session.execute sometimes throws cassandra.Unavailable: Error from server: code=1000 [Unavailable exception] message="Cannot achieve consistency level for cl ALL. Requires 3, alive 2" info={'consistency': 'ALL', 'required_replicas': 3, 'alive_replicas': 2} after one of three nodes is paused, but the rest is addressed

Lorak-mmk

Commit Don't create Host instances with random host_id should be the last one, right? Without test fixes introduced in subsequent commit, this commit can't pass tests I think.

tests/integration/standard/test_query.py

cassandra/cluster.py

cassandra/metadata.py

cassandra/pool.py

… starting point The `test_profile_lb_swap` test logic assumed that `populate` was called before control connection (cc) was created, meaning only the contact points from the cluster configuration were known (a single host). Due to that the starting point was not random. This commit updates the test to reflect the new behavior, where `populate` is called on the load-balancing policy after the control connection is created. This allows the policy to be updated with all known hosts and ensures the starting point is properly randomized.

Previously, the driver relied on the load-balancing policy (LBP) to determine the order of hosts to connect to. Since the default LBP is Round Robin, each reconnection would start from a different host. After removing fake hosts with random IDs at startup, this behavior changed. When the LBP is not yet initialized, the driver now uses the endpoints provided by the control connection (CC), so there is no guarantee that different hosts will be selected on reconnection. This change updates the test logic to first establish a connection and initialize the LBP, and only then verify that two subsequent reconnections land on different hosts in a healthy cluster.

Only compare hosts endpoints not whole Host instances as we don't know hosts ids.

…ive hosts

… local_dc

In DC aware lbp when local_dc is not provided we set it in on_add and it needs to be initialized for distance to give proper results.

Previously, we used endpoints provided to the cluster to create Host instances with random host_ids in order to populate the LBP before the ControlConnection was established. This logic led to creating many connections that were opened and then quickly closed, because once we learned the correct host_ids from system.peers, we removed the old Hosts with random IDs and created new ones with the proper host_ids. This commit introduces a new approach. To establish the ControlConnection, we now use only the resolved contact points from the cluster configuration. Only after a successful connection do we populate Host information in the LBP. If the LBP is already initialized during ControlConnection reconnection, we reuse the existing values.

cassandra/cluster.py

dkropachev · 2026-01-12T15:04:11Z

tests/integration/standard/test_metrics.py

+            with pytest.raises((WriteTimeout, Unavailable)):
                self.session.execute(query, timeout=None)
        finally:
            get_node(1).resume()

        # Change the scales stats_name of the cluster2
        cluster2.metrics.set_stats_name('cluster2-metrics')

        stats_cluster1 = self.cluster.metrics.get_stats()
        stats_cluster2 = cluster2.metrics.get_stats()

        # Test direct access to stats
-        assert 1 == self.cluster.metrics.stats.write_timeouts
+        assert (1 == self.cluster.metrics.stats.write_timeouts or 1 == self.cluster.metrics.stats.unavailables)
        assert 0 == cluster2.metrics.stats.write_timeouts


It is an expected error in given scenario.
Only difference between WriteTimeout and Unavailable is WriteTimeout happens when coordinator scheduled a query not knowing that node is down and not getting response back, while Unavailable is when coordinator knows that node is down and do not even try.

So, reason is why we start getting Unavailable is that a delay between get_node(1).pause() and self.session.execute(query, timeout=None) is increased, which is most-likely has something to do with absence of dropping node pool for fake node records driver used to create before.

I would say it is safe to just add Unavailable to expected exceptions and move on.

sylwiaszunejko requested a review from dkropachev December 18, 2025 13:21

sylwiaszunejko force-pushed the remove_random_ids branch from 7f061e1 to ef382b9 Compare December 18, 2025 13:41

sylwiaszunejko force-pushed the remove_random_ids branch from ef382b9 to 9598dd5 Compare December 18, 2025 14:58

sylwiaszunejko force-pushed the remove_random_ids branch from 9598dd5 to 9e162dd Compare December 19, 2025 13:20

dkropachev requested changes Dec 19, 2025

View reviewed changes

cassandra/cluster.py Show resolved Hide resolved

cassandra/cluster.py Outdated Show resolved Hide resolved

cassandra/policies.py Outdated Show resolved Hide resolved

cassandra/cluster.py Outdated Show resolved Hide resolved

sylwiaszunejko force-pushed the remove_random_ids branch 2 times, most recently from adddec1 to 3e864fc Compare December 20, 2025 12:58

sylwiaszunejko requested a review from dkropachev December 22, 2025 09:57

sylwiaszunejko force-pushed the remove_random_ids branch from 3e864fc to 0a1aa0e Compare December 22, 2025 12:59

sylwiaszunejko self-assigned this Dec 22, 2025

sylwiaszunejko requested a review from Lorak-mmk December 22, 2025 13:21

sylwiaszunejko marked this pull request as ready for review December 22, 2025 13:21

sylwiaszunejko force-pushed the remove_random_ids branch from dd1eb6f to a6cf3aa Compare December 22, 2025 13:56

dkropachev requested changes Dec 23, 2025

View reviewed changes

cassandra/cluster.py Outdated Show resolved Hide resolved

sylwiaszunejko force-pushed the remove_random_ids branch from a6cf3aa to fef57ae Compare December 29, 2025 11:09

sylwiaszunejko requested a review from dkropachev December 29, 2025 11:10

dkropachev reviewed Dec 29, 2025

View reviewed changes

cassandra/cluster.py Outdated Show resolved Hide resolved

sylwiaszunejko force-pushed the remove_random_ids branch from fef57ae to 8124928 Compare December 29, 2025 12:22

dkropachev approved these changes Dec 29, 2025

View reviewed changes

Lorak-mmk requested changes Dec 29, 2025

View reviewed changes

sylwiaszunejko force-pushed the remove_random_ids branch from 8124928 to c381c19 Compare December 29, 2025 15:37

sylwiaszunejko requested a review from Lorak-mmk December 30, 2025 10:40

Lorak-mmk requested changes Dec 30, 2025

View reviewed changes

tests/integration/standard/test_query.py Show resolved Hide resolved

cassandra/cluster.py Show resolved Hide resolved

cassandra/metadata.py Outdated Show resolved Hide resolved

cassandra/pool.py Show resolved Hide resolved

sylwiaszunejko force-pushed the remove_random_ids branch from 2d17c1b to 14f78b5 Compare January 8, 2026 12:29

sylwiaszunejko requested a review from Lorak-mmk January 8, 2026 12:32

sylwiaszunejko force-pushed the remove_random_ids branch 3 times, most recently from 7b7cf1f to 02acb4c Compare January 9, 2026 13:44

sylwiaszunejko added 6 commits January 9, 2026 14:45

Use endpoint instead od Host in _try_connect

cdafe90

tests/integration/standard: don't compare Host instances

f63977f

Only compare hosts endpoints not whole Host instances as we don't know hosts ids.

tests/unit: Provide host_id when initializing Host

5aed786

tests/integration/standard: return empty query plan if there are no l…

9f4334f

…ive hosts

sylwiaszunejko force-pushed the remove_random_ids branch from 02acb4c to fff9753 Compare January 9, 2026 13:46

sylwiaszunejko added 3 commits January 12, 2026 13:30

tests/integration/standard: allow execute to throw Unavailable exception

4692640

Don't check if host is in initial contact points when setting default…

2ec1d2e

… local_dc

Call on_add before distance to properly initialize lbp

79abbec

In DC aware lbp when local_dc is not provided we set it in on_add and it needs to be initialized for distance to give proper results.

sylwiaszunejko force-pushed the remove_random_ids branch from fff9753 to c0e9f73 Compare January 12, 2026 12:30

sylwiaszunejko force-pushed the remove_random_ids branch from c0e9f73 to 2beeb75 Compare January 12, 2026 13:35

dkropachev approved these changes Jan 12, 2026

View reviewed changes

Lorak-mmk approved these changes Jan 14, 2026

View reviewed changes

dkropachev merged commit a0cde2e into scylladb:master Jan 14, 2026
21 checks passed

Don't create Host instances with random host_id #623

Don't create Host instances with random host_id #623

Uh oh!

Conversation

sylwiaszunejko commented Dec 18, 2025 • edited by dkropachev Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Uh oh!

sylwiaszunejko commented Dec 18, 2025

Uh oh!

sylwiaszunejko commented Dec 18, 2025

Uh oh!

Lorak-mmk commented Dec 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lorak-mmk commented Dec 18, 2025

Uh oh!

sylwiaszunejko commented Dec 18, 2025

Uh oh!

Lorak-mmk commented Dec 18, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sylwiaszunejko commented Dec 19, 2025

Uh oh!

Lorak-mmk commented Dec 19, 2025

Uh oh!

Lorak-mmk commented Dec 19, 2025

Uh oh!

sylwiaszunejko commented Dec 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Lorak-mmk commented Dec 29, 2025

Uh oh!

Uh oh!

Uh oh!

Lorak-mmk Dec 29, 2025

Choose a reason for hiding this comment

Uh oh!

dkropachev Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sylwiaszunejko commented Dec 30, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Lorak-mmk left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

dkropachev Jan 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

sylwiaszunejko commented Dec 18, 2025 •

edited by dkropachev

Loading

Lorak-mmk commented Dec 18, 2025 •

edited

Loading

sylwiaszunejko commented Dec 19, 2025 •

edited

Loading

dkropachev Jan 12, 2026 •

edited

Loading

sylwiaszunejko commented Dec 30, 2025 •

edited

Loading

dkropachev Jan 12, 2026 •

edited

Loading