diff --git a/src/Cassandra.Tests/HostConnectionPoolTests.cs b/src/Cassandra.Tests/HostConnectionPoolTests.cs index 522842424..2ddae8b34 100644 --- a/src/Cassandra.Tests/HostConnectionPoolTests.cs +++ b/src/Cassandra.Tests/HostConnectionPoolTests.cs @@ -330,6 +330,130 @@ public void OnHostUp_Does_Not_Recreates_Pool_For_Ignored_Hosts() Assert.AreEqual(0, Volatile.Read(ref creationCounter)); } + [Test] + public async Task OnDistanceChanged_To_Ignored_On_Empty_Pool_Should_Not_Block_Later_Reconnection() + { + // Reproduces the case where a host loses its tokens (becomes Ignored) while its pool is empty + // and later gains them back. Draining an empty pool must still transition the pool state back + // to Init; otherwise it stays Closing and IsClosing blocks the reconnection when the host + // becomes routable again, leaving the node permanently unusable. + var host = TestHelper.CreateHost("127.0.0.1"); + _mock = GetPoolMock(host, GetConfig(1, 1, new ConstantReconnectionPolicy(3600000))); + var allowCreation = 0; + _mock.Setup(p => p.DoCreateAndOpen(It.IsAny(), -1, 0, 0)).Returns(() => + { + if (Volatile.Read(ref allowCreation) == 0) + { + return TaskHelper.FromException(new Exception("Test Exception")); + } + return TaskHelper.ToTask(CreateConnection()); + }); + var pool = _mock.Object; + + // Move the empty pool to Local (creation fails, so the pool stays empty) and then to Ignored, + // which drains the (empty) pool. + host.SetDistance(HostDistance.Local); + host.SetDistance(HostDistance.Ignored); + Assert.AreEqual(0, pool.OpenConnections); + + // The host gains tokens again: allow connections and bring it back to Local. + Volatile.Write(ref allowCreation, 1); + host.SetDistance(HostDistance.Local); + + await TestHelper.WaitUntilAsync(() => pool.OpenConnections == 1).ConfigureAwait(false); + Assert.AreEqual(1, pool.OpenConnections); + } + + [Test] + public async Task OnDistanceChanged_To_Ignored_Should_Not_Add_Connection_From_InFlight_Open() + { + // A connection open can be in progress when the host becomes Ignored (for example a zero-token + // node). Draining the (still empty) pool must not reset its state while that open is in flight, + // otherwise the completing open observes a non-closing pool and adds a connection to the + // ignored host. + var host = TestHelper.CreateHost("127.0.0.1"); + _mock = GetPoolMock(host, GetConfig(1, 1, new ConstantReconnectionPolicy(3600000))); + var openGate = new TaskCompletionSource(); + var openStarted = new TaskCompletionSource(); + _mock.Setup(p => p.DoCreateAndOpen(It.IsAny(), -1, 0, 0)).Returns(async () => + { + openStarted.TrySetResult(true); + await openGate.Task.ConfigureAwait(false); + return CreateConnection(); + }); + var pool = _mock.Object; + + // Start opening a connection and wait until it is in flight (blocked on the gate). + host.SetDistance(HostDistance.Local); + await openStarted.Task.ConfigureAwait(false); + + // The host becomes ignored while the open is still in progress; this drains the empty pool. + host.SetDistance(HostDistance.Ignored); + + // Let the in-flight open complete. It must observe the closing/ignored state and discard its + // connection instead of adding it to the pool. + openGate.SetResult(true); + + // Poll for the incorrect behaviour (a connection being added). This returns as soon as the bug + // is observed, otherwise it settles after the full window and we assert nothing was added. + await TestHelper.WaitUntilAsync(() => pool.OpenConnections == 1, 50, 10).ConfigureAwait(false); + Assert.AreEqual(0, pool.OpenConnections); + } + + [Test] + public async Task OnDistanceChanged_To_Ignored_Then_Local_During_FinishEmptyDrain_Should_Reconnect() + { + // Reproduces the race where a host becomes Ignored (empty pool, in-flight open) and then + // immediately becomes Local again while FinishEmptyDrain is still polling. + // + // Timeline: + // 1. pool empty; connection open in flight (blocked on gate) + // 2. host → Ignored: pool state Closing, FinishEmptyDrain polls (open still in flight) + // 3. host → Local: OnDistanceChanged fires ScheduleReconnection but IsClosing is true → skipped + // 4. in-flight open finishes: pool is Closing so connection is discarded; _connectionOpenTcs cleared + // 5. FinishEmptyDrain retry: _connectionOpenTcs == null → afterDrainHandler() → state = Init + // → ScheduleReconnection triggered by FinishEmptyDrain to recover from the missed call in step 3 + // 6. pool should reconnect and reach OpenConnections == 1 + var host = TestHelper.CreateHost("127.0.0.1"); + _mock = GetPoolMock(host, GetConfig(1, 1, new ConstantReconnectionPolicy(50))); + var openGate = new TaskCompletionSource(); + var openStarted = new TaskCompletionSource(); + var callCount = 0; + _mock.Setup(p => p.DoCreateAndOpen(It.IsAny(), -1, 0, 0)).Returns(async () => + { + var call = Interlocked.Increment(ref callCount); + if (call == 1) + { + // First open: block until the gate is released (the in-flight open that gets discarded). + openStarted.TrySetResult(true); + await openGate.Task.ConfigureAwait(false); + return CreateConnection(); + } + // Subsequent opens: succeed immediately so the pool can reconnect. + return CreateConnection(); + }); + var pool = _mock.Object; + + // Start the first open and wait until it is in flight. + host.SetDistance(HostDistance.Local); + await openStarted.Task.ConfigureAwait(false); + + // Host becomes Ignored: pool → Closing; FinishEmptyDrain polls because open is still in flight. + host.SetDistance(HostDistance.Ignored); + + // Host regains tokens: distance updated to Local while pool is still Closing. + // OnDistanceChanged calls ScheduleReconnection but CreateOrScheduleReconnectAsync returns early. + host.SetDistance(HostDistance.Local); + + // Release the in-flight open. The pool is still Closing so the connection is discarded. + openGate.SetResult(true); + + // FinishEmptyDrain should eventually complete (state → Init) and call ScheduleReconnection + // to recover the missed reconnect. The pool must reach OpenConnections == 1. + await TestHelper.WaitUntilAsync(() => pool.OpenConnections == 1, 200, 20).ConfigureAwait(false); + Assert.AreEqual(1, pool.OpenConnections); + } + [Test] public async Task EnsureCreate_After_Reconnection_Attempt_Waits_Existing() { diff --git a/src/Cassandra.Tests/HostTests.cs b/src/Cassandra.Tests/HostTests.cs index 2e45e2d64..b97c1fda3 100644 --- a/src/Cassandra.Tests/HostTests.cs +++ b/src/Cassandra.Tests/HostTests.cs @@ -53,7 +53,106 @@ public void Should_UseHostIdEmpty_When_HostIdIsNull() Assert.AreEqual(Guid.Empty, host.HostId); } - private IRow BuildRow(Guid? hostId) + [Test] + public void SetInfo_Should_SetDistanceToIgnored_When_HostBecomesZeroTokenNode() + { + var host = new Host(Address, contactPoint: null); + var distanceChanges = new List>(); + host.DistanceChanged += (previous, current) => distanceChanges.Add(Tuple.Create(previous, current)); + + host.SetInfo(BuildRow(Guid.NewGuid())); + host.SetDistance(HostDistance.Local); + host.SetInfo(BuildRow(Guid.NewGuid(), new string[0])); + + Assert.AreEqual(HostDistance.Ignored, host.GetDistanceUnsafe()); + Assert.AreEqual(2, distanceChanges.Count); + Assert.AreEqual(Tuple.Create(HostDistance.Ignored, HostDistance.Local), distanceChanges[0]); + Assert.AreEqual(Tuple.Create(HostDistance.Local, HostDistance.Ignored), distanceChanges[1]); + } + + [Test] + public void SetInfo_Should_NotBringHostUp_When_ZeroTokenNodeGainsTokens() + { + var host = new Host(Address, contactPoint: null); + var upCount = 0; + host.Up += _ => Interlocked.Increment(ref upCount); + + // Host is reported as a zero-token node and then marked as DOWN (as a status change event + // would do for an ignored, non-routable host that has no pool). + host.SetInfo(BuildRow(Guid.NewGuid(), new string[0])); + host.SetDown(); + Assert.IsFalse(host.IsUp); + + // A later topology refresh gives the host tokens. SetInfo must NOT call BringUpIfDown(): + // a token update does not prove liveness — system.peers can still contain a down peer. + // The connection pool's successful open will call BringUpIfDown() once a real connection + // confirms the node is reachable. + host.SetInfo(BuildRow(Guid.NewGuid(), new[] { "1" })); + + Assert.IsFalse(host.IsZeroTokenNode); + Assert.IsFalse(host.IsUp); + Assert.AreEqual(0, upCount); + } + + [Test] + public void SetInfo_Should_NotBringHostUp_When_HostAlreadyHadTokens() + { + var host = new Host(Address, contactPoint: null); + var upCount = 0; + host.Up += _ => Interlocked.Increment(ref upCount); + + host.SetInfo(BuildRow(Guid.NewGuid(), new[] { "1" })); + host.SetDown(); + Assert.IsFalse(host.IsUp); + + // Refreshing token metadata for a host that was never a zero-token node must not resurrect it. + host.SetInfo(BuildRow(Guid.NewGuid(), new[] { "2" })); + + Assert.IsFalse(host.IsUp); + Assert.AreEqual(0, upCount); + } + + [Test] + public void SetInfo_Should_MarkHostAsZeroTokenNode_When_TokensColumnIsNull() + { + // Scylla returns the empty non-frozen tokens set as NULL in system.local/system.peers, so a + // present-but-NULL tokens column identifies a real zero-token node. + var host = new Host(Address, contactPoint: null); + var row = new TestHelper.DictionaryBasedRow(new Dictionary + { + { "host_id", Guid.NewGuid() }, + { "data_center", "dc1" }, + { "rack", "rack1" }, + { "release_version", "3.11.1" }, + { "tokens", null } + }); + + host.SetInfo(row); + + Assert.IsTrue(host.IsZeroTokenNode); + Assert.AreEqual(HostDistance.Ignored, host.GetDistanceUnsafe()); + } + + [Test] + public void SetInfo_Should_NotMarkHostAsZeroTokenNode_When_TokensColumnIsAbsent() + { + // A row that does not carry the tokens column (the column was not selected) must not turn the + // host into a zero-token node; its token state is simply unknown. + var host = new Host(Address, contactPoint: null); + var row = new TestHelper.DictionaryBasedRow(new Dictionary + { + { "host_id", Guid.NewGuid() }, + { "data_center", "dc1" }, + { "rack", "rack1" }, + { "release_version", "3.11.1" } + }); + + host.SetInfo(row); + + Assert.IsFalse(host.IsZeroTokenNode); + } + + private IRow BuildRow(Guid? hostId, IEnumerable tokens = null) { return new TestHelper.DictionaryBasedRow(new Dictionary { @@ -61,7 +160,7 @@ private IRow BuildRow(Guid? hostId) { "data_center", "dc1"}, { "rack", "rack1" }, { "release_version", "3.11.1" }, - { "tokens", new List { "1" }} + { "tokens", tokens ?? new List { "1" }} }); } } diff --git a/src/Cassandra.Tests/Policies/DefaultLoadBalancingTests.cs b/src/Cassandra.Tests/Policies/DefaultLoadBalancingTests.cs index 659bde13b..f8d4f7768 100644 --- a/src/Cassandra.Tests/Policies/DefaultLoadBalancingTests.cs +++ b/src/Cassandra.Tests/Policies/DefaultLoadBalancingTests.cs @@ -63,5 +63,41 @@ public void Should_Set_Distance_For_Preferred_Host_To_Local() lbp.NewQueryPlan(null, statement); Assert.AreEqual(HostDistance.Local, lbp.Distance(statement.PreferredHost)); } + + [Test] + public void Should_Return_Ignored_Distance_For_Zero_Token_Preferred_Host() + { + // A zero-token node must remain Ignored even after being set as the preferred host. +#pragma warning disable 618 + var lbp = new DefaultLoadBalancingPolicy(new TestLoadBalancingPolicy()); +#pragma warning restore 618 + var zeroTokenHost = TestHelper.CreateHost("0.0.0.5", tokens: new string[0]); + Assert.IsTrue(zeroTokenHost.IsZeroTokenNode); + + var statement = new TargettedSimpleStatement("Q"); + statement.PreferredHost = zeroTokenHost; + lbp.NewQueryPlan(null, statement); + + Assert.AreEqual(HostDistance.Ignored, lbp.Distance(zeroTokenHost)); + } + + [Test] + public void Should_Not_Yield_Zero_Token_Preferred_Host_In_Query_Plan() + { + // A zero-token preferred host must not appear in the query plan. +#pragma warning disable 618 + var lbp = new DefaultLoadBalancingPolicy(new TestLoadBalancingPolicy()); +#pragma warning restore 618 + var zeroTokenHost = TestHelper.CreateHost("0.0.0.5", tokens: new string[0]); + Assert.IsTrue(zeroTokenHost.IsZeroTokenNode); + + var statement = new TargettedSimpleStatement("Q"); + statement.PreferredHost = zeroTokenHost; + var hosts = lbp.NewQueryPlan(null, statement).Select(h => h.Host).ToList(); + + CollectionAssert.DoesNotContain(hosts, zeroTokenHost); + // Child policy hosts are still returned. + Assert.AreEqual(2, hosts.Count); + } } } diff --git a/src/Cassandra.Tests/PoliciesUnitTests.cs b/src/Cassandra.Tests/PoliciesUnitTests.cs index 378b164d9..5a3706909 100644 --- a/src/Cassandra.Tests/PoliciesUnitTests.cs +++ b/src/Cassandra.Tests/PoliciesUnitTests.cs @@ -120,6 +120,33 @@ public void RoundRobinIsCyclicTestInParallel() clusterMock.Verify(); } + [Test] + public void RoundRobinPolicySkipsZeroTokenHosts() + { + var hostList = new List + { + TestHelper.CreateHost("0.0.0.1", tokens: new[] { "1" }), + TestHelper.CreateHost("0.0.0.2", tokens: new string[0]), + TestHelper.CreateHost("0.0.0.3", tokens: new[] { "3" }) + }; + + var clusterMock = new Mock(); + clusterMock + .Setup(c => c.AllHosts()) + .Returns(hostList) + .Verifiable(); + + var policy = new RoundRobinPolicy(); + policy.Initialize(clusterMock.Object); + + CollectionAssert.AreEquivalent( + new byte[] { 1, 3 }, + policy.NewQueryPlan(null, new SimpleStatement()).Select(h => TestHelper.GetLastAddressByte(h.Host))); + Assert.AreEqual(HostDistance.Ignored, policy.Distance(hostList[1])); + Assert.AreEqual(HostDistance.Local, policy.Distance(hostList[0])); + clusterMock.Verify(); + } + [Test] public void DCAwareRoundRobinPolicyNeverHitsRemoteWhenSet() { @@ -160,6 +187,106 @@ public void DCAwareRoundRobinPolicyNeverHitsRemoteWhenSet() Assert.AreEqual(0, followingRounds.Count(h => h.Host.Datacenter != "local")); } + [Test] + public void DCAwareRoundRobinPolicySkipsZeroTokenHosts() + { + var hostList = new List + { + TestHelper.CreateHost("0.0.0.1", "dc1", tokens: new[] { "1" }), + TestHelper.CreateHost("0.0.0.2", "dc1", tokens: new string[0]), + TestHelper.CreateHost("0.0.0.3", "dc2", tokens: new[] { "3" }), + TestHelper.CreateHost("0.0.0.4", "dc2", tokens: new string[0]) + }; + var clusterMock = new Mock(); + clusterMock + .Setup(c => c.AllHosts()) + .Returns(hostList); + + var policy = new DCAwareRoundRobinPolicy("dc1", 1); + policy.Initialize(clusterMock.Object); + + CollectionAssert.AreEqual( + new byte[] { 1, 3 }, + policy.NewQueryPlan(null, new SimpleStatement()).Select(h => TestHelper.GetLastAddressByte(h.Host))); + Assert.AreEqual(HostDistance.Ignored, policy.Distance(hostList[1])); + Assert.AreEqual(HostDistance.Ignored, policy.Distance(hostList[3])); + Assert.AreEqual(HostDistance.Local, policy.Distance(hostList[0])); + Assert.AreEqual(HostDistance.Remote, policy.Distance(hostList[2])); + } + + [Test] + public void DCAwareRoundRobinPolicyRoutesHostThatGainsTokens() + { + var host = TestHelper.CreateHost("0.0.0.1", "dc1", tokens: new string[0]); + var hostList = new List + { + host, + TestHelper.CreateHost("0.0.0.2", "dc1", tokens: new[] { "2" }) + }; + var clusterMock = new Mock(); + clusterMock + .Setup(c => c.AllHosts()) + .Returns(hostList); + + var policy = new DCAwareRoundRobinPolicy("dc1", 0); + policy.Initialize(clusterMock.Object); + + CollectionAssert.AreEqual( + new byte[] { 2 }, + policy.NewQueryPlan(null, new SimpleStatement()).Select(h => TestHelper.GetLastAddressByte(h.Host))); + + host.SetInfo(new TestHelper.DictionaryBasedRow(new Dictionary + { + { "data_center", "dc1" }, + { "rack", "rack1" }, + { "tokens", new[] { "1" } } + })); + + CollectionAssert.AreEquivalent( + new byte[] { 1, 2 }, + policy.NewQueryPlan(null, new SimpleStatement()).Select(h => TestHelper.GetLastAddressByte(h.Host))); + Assert.AreEqual(HostDistance.Local, policy.Distance(host)); + } + + [Test] + public void DCAwareRoundRobinPolicyBalancesFirstHostAcrossRoutableLocalHosts() + { + //A zero-token node precedes the routable hosts. The rotation must be balanced across the + //routable hosts only, not skewed towards the host that follows the zero-token node. + var hostList = new List + { + TestHelper.CreateHost("0.0.0.1", "dc1", tokens: new string[0]), + TestHelper.CreateHost("0.0.0.2", "dc1", tokens: new[] { "2" }), + TestHelper.CreateHost("0.0.0.3", "dc1", tokens: new[] { "3" }) + }; + var clusterMock = new Mock(); + clusterMock + .Setup(c => c.AllHosts()) + .Returns(hostList); + + var policy = new DCAwareRoundRobinPolicy("dc1", 0); + policy.Initialize(clusterMock.Object); + + var firstHostCounts = new Dictionary(); + const int iterations = 1000; + for (var i = 0; i < iterations; i++) + { + var plan = policy.NewQueryPlan(null, new SimpleStatement()) + .Select(h => TestHelper.GetLastAddressByte(h.Host)) + .ToArray(); + //The zero-token host is never part of the plan + CollectionAssert.DoesNotContain(plan, (byte)1); + CollectionAssert.AreEquivalent(new byte[] { 2, 3 }, plan); + var first = plan[0]; + firstHostCounts.TryGetValue(first, out var count); + firstHostCounts[first] = count + 1; + } + + //Both routable hosts must receive the same number of first attempts + Assert.AreEqual(iterations / 2, firstHostCounts[2]); + Assert.AreEqual(iterations / 2, firstHostCounts[3]); + } + [Test] public void DCAwareRoundRobinYieldsRemoteNodesAtTheEnd() { diff --git a/src/Cassandra.Tests/Requests/PrepareHandlerTests.cs b/src/Cassandra.Tests/Requests/PrepareHandlerTests.cs index b7652093e..8a0573f78 100644 --- a/src/Cassandra.Tests/Requests/PrepareHandlerTests.cs +++ b/src/Cassandra.Tests/Requests/PrepareHandlerTests.cs @@ -478,6 +478,101 @@ await mockResult.PrepareHandler.Prepare( } } + [Test] + public async Task Should_CreateConnectionPoolForHost_When_ZeroTokenHostGainsTokens() + { + var lbp = new RoundRobinPolicy(); + var mockResult = BuildPrepareHandler( + builder => + { + builder.QueryOptions = + new QueryOptions() + .SetConsistencyLevel(ConsistencyLevel.LocalOne) + .SetSerialConsistencyLevel(ConsistencyLevel.LocalSerial); + builder.SocketOptions = + new SocketOptions().SetReadTimeoutMillis(10); + builder.Policies = new Cassandra.Policies( + lbp, + new ConstantReconnectionPolicy(5), + new DefaultRetryPolicy(), + NoSpeculativeExecutionPolicy.Instance, + new AtomicMonotonicTimestampGenerator(), + null); + }); + // mock connection send + mockResult.ConnectionFactory.OnCreate += connection => + { + Mock.Get(connection) + .Setup(c => c.Send(It.IsAny())) + .Returns(async req => + { + mockResult.SendResults.Enqueue(new ConnectionSendResult { Connection = connection, Request = req }); + await Task.Delay(1).ConfigureAwait(false); + return new ProxyResultResponse( + ResultResponse.ResultResponseKind.Void, + new OutputPrepared( + new byte[0], new RowSetMetadata { Columns = new CqlColumn[0] }, new RowSetMetadata { Columns = new CqlColumn[0] })); + }); + }; + + var zeroTokenAddress = new IPEndPoint(IPAddress.Parse("127.0.0.2"), 9042); + var host = mockResult.Session.InternalCluster.AllHosts().Single(h => h.Address.Equals(zeroTokenAddress)); + var request = new InternalPrepareRequest(_serializer, "TEST", null, null); + + // The host is reported as a zero-token node: it must be ignored, excluded from the query plan + // and the session must not create a pool for it even when it is placed first in the plan. + host.SetInfo(BuildHostRow(new string[0])); + Assert.IsTrue(host.IsZeroTokenNode); + Assert.AreEqual(HostDistance.Ignored, mockResult.Session.InternalCluster.RetrieveAndSetDistance(host)); + Assert.IsFalse( + lbp.NewQueryPlan(null, new SimpleStatement()).Select(h => h.Host.Address).Contains(zeroTokenAddress)); + + await mockResult.PrepareHandler.Prepare( + request, + mockResult.Session, + BuildQueryPlanWithHostFirst(mockResult, zeroTokenAddress)).ConfigureAwait(false); + Assert.IsNull(mockResult.Session.GetExistingPool(zeroTokenAddress)); + + // The host now owns tokens: it must be routable again and the session must create a pool for it. + host.SetInfo(BuildHostRow(new[] { "2" })); + Assert.IsFalse(host.IsZeroTokenNode); + Assert.AreEqual(HostDistance.Local, mockResult.Session.InternalCluster.RetrieveAndSetDistance(host)); + Assert.IsTrue( + lbp.NewQueryPlan(null, new SimpleStatement()).Select(h => h.Host.Address).Contains(zeroTokenAddress)); + + await mockResult.PrepareHandler.Prepare( + request, + mockResult.Session, + BuildQueryPlanWithHostFirst(mockResult, zeroTokenAddress)).ConfigureAwait(false); + + var pool = mockResult.Session.GetExistingPool(zeroTokenAddress); + Assert.IsNotNull(pool); + Assert.IsTrue(pool.HasConnections); + } + + private static IEnumerator BuildQueryPlanWithHostFirst(PrepareHandlerMockResult mockResult, IPEndPoint firstHostAddress) + { + var hosts = mockResult.Session.InternalCluster.AllHosts(); + var plan = new List + { + new HostShard(hosts.Single(h => h.Address.Equals(firstHostAddress)), -1) + }; + plan.AddRange(hosts.Where(h => !h.Address.Equals(firstHostAddress)).Select(h => new HostShard(h, -1))); + return plan.GetEnumerator(); + } + + private static IRow BuildHostRow(IEnumerable tokens) + { + return new TestHelper.DictionaryBasedRow(new Dictionary + { + { "host_id", Guid.NewGuid() }, + { "data_center", "dc1" }, + { "rack", "rack1" }, + { "release_version", "3.11.1" }, + { "tokens", tokens } + }); + } + private PrepareHandlerMockResult BuildPrepareHandler(Action configBuilderAct) { var factory = new FakeConnectionFactory(MockConnection); diff --git a/src/Cassandra.Tests/TestHelper.cs b/src/Cassandra.Tests/TestHelper.cs index eac37cf0b..1b2f548a8 100644 --- a/src/Cassandra.Tests/TestHelper.cs +++ b/src/Cassandra.Tests/TestHelper.cs @@ -151,13 +151,20 @@ public static Host CreateHost(string address, string dc = "dc1", string rack = " { var h = new Host(new IPEndPoint(IPAddress.Parse(address), ProtocolOptions.DefaultPort), new ConstantReconnectionPolicy(1)); - h.SetInfo(new DictionaryBasedRow(new Dictionary + var values = new Dictionary { { "data_center", dc }, { "rack", rack }, - { "tokens", tokens }, { "release_version", cassandraVersion }, - })); + }; + // A null tokens argument models a row that does not carry token information (the column was + // not selected), so the host keeps its default, non-zero-token state. Passing an empty + // collection models a real zero-token node (an empty/NULL tokens column). + if (tokens != null) + { + values["tokens"] = tokens; + } + h.SetInfo(new DictionaryBasedRow(values)); return h; } diff --git a/src/Cassandra/Cluster.cs b/src/Cassandra/Cluster.cs index 0b1517921..a2565e83a 100644 --- a/src/Cassandra/Cluster.cs +++ b/src/Cassandra/Cluster.cs @@ -605,6 +605,18 @@ public async Task ShutdownAsync(int timeoutMs = Timeout.Infinite) /// HostDistance IInternalCluster.RetrieveAndSetDistance(Host host) { + // Zero-token nodes must never be routed to, regardless of the configured + // load balancing policy. Enforcing it here — the single choke point every code path funnels + // through (request handler, prepare handler, control connection, session warmup) — guarantees + // the exclusion holds even for custom policies that do not check Host.IsZeroTokenNode, and keeps + // the distance consistent with the Ignored value that Host.SetInfo forces when the node is + // first observed as zero-token. + if (host.IsZeroTokenNode) + { + host.SetDistance(HostDistance.Ignored); + return HostDistance.Ignored; + } + var distance = _loadBalancingPolicies[0].Distance(host); for (var i = 1; i < _loadBalancingPolicies.Count; i++) diff --git a/src/Cassandra/Connections/HostConnectionPool.cs b/src/Cassandra/Connections/HostConnectionPool.cs index aca623808..61c7a061b 100644 --- a/src/Cassandra/Connections/HostConnectionPool.cs +++ b/src/Cassandra/Connections/HostConnectionPool.cs @@ -37,6 +37,10 @@ internal class HostConnectionPool : IHostConnectionPool private Random rand = new Random(); private const int ConnectionIndexOverflow = int.MaxValue - 1000000; private const long BetweenResizeDelay = 2000; + private const long EmptyDrainRecheckMillis = 1000; + // Maximum number of 1-second polls in FinishEmptyDrain before forcing completion. + // Mirrors the spirit of the DrainConnectionsTimer steps cap. + private const int EmptyDrainMaxRetries = 30; /// /// Represents the possible states of the pool. @@ -107,6 +111,15 @@ private static class PoolState /// private bool IsClosing => Volatile.Read(ref _state) != PoolState.Init; + /// + /// True when a freshly opened connection must not be kept: either the pool is shutting + /// down/draining, or the host is . Checking + /// in addition to the pool state closes the race where the empty-drain timeout resets the pool + /// state back to while a connection open is still in flight; an + /// ignored host must never end up with an established connection. + /// + private bool ShouldDiscardNewConnection => IsClosing || _distance == HostDistance.Ignored; + /// public IConnection[] ConnectionsSnapshot => _connections.GetSnapshot().GetAllItems(); @@ -538,6 +551,7 @@ private void DrainConnections(Action afterDrainHandler) if (connections.Length == 0) { HostConnectionPool.Logger.Info("Pool #{0} to {1} had no connections", GetHashCode(), _host.Address); + FinishEmptyDrain(afterDrainHandler); return; } // The request handler might execute up to 2 queries with a single connection: @@ -552,6 +566,53 @@ private void DrainConnections(Action afterDrainHandler) DrainConnectionsTimer(connections, afterDrainHandler, delay / 1000); } + /// + /// Completes the draining of a pool that has no established connections. It waits for any connection + /// open that is still in progress before invoking the handler, so the pool state is not reset while + /// an open could still add a connection to an ignored host. + /// + private void FinishEmptyDrain(Action afterDrainHandler, int retriesLeft = HostConnectionPool.EmptyDrainMaxRetries) + { + if (Volatile.Read(ref _connectionOpenTcs) == null || retriesLeft <= 0) + { + if (retriesLeft <= 0) + { + HostConnectionPool.Logger.Warning( + "Pool #{0} to {1}: in-flight connection open did not complete within {2} s; " + + "forcing drain completion.", + GetHashCode(), _host.Address, HostConnectionPool.EmptyDrainMaxRetries); + } + // No open in flight (or we gave up waiting), safe to complete the drain (which resets the pool state). + CompleteDrain(afterDrainHandler); + return; + } + // A connection open is still in progress. Keep the pool in Closing and retry shortly so the + // in-flight open finishes and discards its connection before the state is reset. + HostConnectionPool.Logger.Info( + "Pool #{0} to {1} waiting for an in-flight connection open before finishing drain", + GetHashCode(), _host.Address); + _timer.NewTimeout(_ => Task.Run(() => FinishEmptyDrain(afterDrainHandler, retriesLeft - 1)), null, HostConnectionPool.EmptyDrainRecheckMillis); + } + + /// + /// Runs the after-drain handler (which resets the pool state) and then recovers a reconnection that + /// may have been missed while the pool was Closing. If OnDistanceChanged(Ignored → Local/Remote) + /// fired during the drain, its ScheduleReconnection call returned early because IsClosing was true + /// at that point; now that the state has been reset to Init we kick off reconnection so the pool is + /// not left idle. + /// + private void CompleteDrain(Action afterDrainHandler) + { + afterDrainHandler?.Invoke(); + if (!IsClosing && _distance != HostDistance.Ignored) + { + // Mirror OnDistanceChanged / OnHostUp: reset _canCreateForeground before reconnecting + // so that foreground EnsureCreate calls don't fail while the reconnection is pending. + _canCreateForeground = true; + ScheduleReconnection(true); + } + } + private void DrainConnectionsTimer(ShardedList connections, Action afterDrainHandler, int steps) { _timer.NewTimeout(_ => @@ -572,7 +633,7 @@ private void DrainConnectionsTimer(ShardedList connections, Action { c.Dispose(); } - afterDrainHandler?.Invoke(); + CompleteDrain(afterDrainHandler); }); }, null, 1000); } @@ -799,7 +860,7 @@ private async Task CreateOpenConnection(bool satisfyWithAnOpenConne return await FinishOpen(tcs, true, ex).ConfigureAwait(false); } - if (IsClosing) + if (ShouldDiscardNewConnection) { HostConnectionPool.Logger.Info("Connection to {0} opened successfully but pool #{1} was being closed", _host.Address, GetHashCode()); @@ -811,7 +872,7 @@ private async Task CreateOpenConnection(bool satisfyWithAnOpenConne HostConnectionPool.Logger.Info("Connection to {0} opened successfully, pool #{1} length: {2}", _host.Address, GetHashCode(), newLength); - if (IsClosing) + if (ShouldDiscardNewConnection) { // We haven't use a CAS operation, so it's possible that the pool is being closed while adding a new // connection, we should remove it. diff --git a/src/Cassandra/Host.cs b/src/Cassandra/Host.cs index 2ff6a9727..bd75a6884 100644 --- a/src/Cassandra/Host.cs +++ b/src/Cassandra/Host.cs @@ -16,6 +16,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Net; using System.Threading; using Cassandra.Connections; @@ -32,7 +33,7 @@ public class Host : IEquatable private static readonly Logger Logger = new Logger(typeof(Host)); private long _isUpNow = 1; private int _distance = (int)HostDistance.Ignored; - private static readonly IReadOnlyCollection WorkloadsDefault = new string[0]; + private static readonly IReadOnlyCollection WorkloadsDefault = Array.Empty(); /// /// Event that gets raised when the host is set as DOWN (not available) by the driver, after being UP. @@ -87,6 +88,17 @@ public bool IsConsiderablyUp /// internal IEnumerable Tokens { get; private set; } + /// + /// true if the node is a zero-token node — i.e. the + /// tokens column was present in the topology row and contained an empty + /// (or NULL) token set. false when the host owns tokens, or when no token + /// metadata has been received yet (the column was absent from the row). + /// + internal bool IsZeroTokenNode => _isZeroTokenNode; + + // Cached in SetInfo so query-path readers get an atomic value and avoid enumerating Tokens. + private volatile bool _isZeroTokenNode; + /// /// Gets the name of the datacenter this host is part of. The returned /// datacenter name is the one as known by Cassandra. Also note that it is @@ -128,6 +140,7 @@ internal Host(IPEndPoint address, IContactPoint contactPoint) { Address = address ?? throw new ArgumentNullException(nameof(address)); ContactPoint = contactPoint; + Tokens = Array.Empty(); } /// @@ -171,11 +184,42 @@ public void SetAsRemoved() /// /// Sets datacenter, rack and other basic information of a host. /// + // NOTE: SetInfo is called only from the topology-refresh loop, which runs on a single scheduler + // thread. Concurrent calls are not expected; the token block below is therefore not locked. internal void SetInfo(IRow row) { Datacenter = row.GetValue("data_center"); Rack = row.GetValue("rack"); - Tokens = row.GetValue>("tokens") ?? new string[0]; + if (row.ContainsColumn("tokens")) + { + // When the tokens column is selected it is authoritative: a NULL value means the node + // advertises an empty token set (Scylla returns the empty non-frozen set as NULL in + // system.local/system.peers), i.e. a real zero-token node. Only skip the update when the + // column is absent, which means token information was not requested/available. + var tokens = row.IsNull("tokens") + ? Array.Empty() + : row.GetValue>("tokens") ?? Array.Empty(); + // Materialize once: Tokens is consumed again later (e.g. TokenMap) and _isZeroTokenNode + // needs the count, so avoid enumerating a potentially lazy sequence more than once. + var tokenList = tokens as ICollection ?? tokens.ToArray(); + Tokens = tokenList; + _isZeroTokenNode = tokenList.Count == 0; + if (_isZeroTokenNode) + { + SetDistance(HostDistance.Ignored); + } + // Distance recovery on the reverse edge (a former zero-token node that gains tokens) is + // intentionally lazy: we do NOT fire SetDistance/DistanceChanged here. Once _isZeroTokenNode + // is false again, IInternalCluster.RetrieveAndSetDistance stops forcing Ignored and the + // configured load balancing policy decides the distance; that call happens on the next query + // plan / prepare / control-connection refresh, which re-includes the host and creates its + // pool. See PrepareHandlerTests.Should_CreateConnectionPoolForHost_When_ZeroTokenHostGainsTokens. + // We also do not call BringUpIfDown() here: a token update does not prove liveness — + // system.peers can still contain a down peer — so marking the host UP before any + // successful connection would trigger query routing and Up subscribers prematurely. The + // connection pool's successful open will call BringUpIfDown() once a real connection + // confirms the node is reachable. + } if (row.ContainsColumn("release_version")) { diff --git a/src/Cassandra/Policies/DCAwareRoundRobinPolicy.cs b/src/Cassandra/Policies/DCAwareRoundRobinPolicy.cs index d66b17a9d..8d13b2dfb 100644 --- a/src/Cassandra/Policies/DCAwareRoundRobinPolicy.cs +++ b/src/Cassandra/Policies/DCAwareRoundRobinPolicy.cs @@ -173,16 +173,24 @@ private Host GetLocalHost() } /// - /// Return the HostDistance for the provided host.

This policy consider nodes - /// in the local datacenter as Local. For each remote datacenter, it - /// considers a configurable number of hosts as Remote and the rest - /// is Ignored.

To configure how many host in each remote - /// datacenter is considered Remote.

+ /// Return the HostDistance for the provided host. This policy considers nodes + /// in the local datacenter as Local and all other non-zero-token nodes as + /// Remote. The usedHostsPerRemoteDc limit controls how many remote + /// hosts appear in a query plan, but does not affect the distance returned here. + /// Zero-token nodes are always + /// regardless of their datacenter and are never included in a query plan. ///
/// the host of which to return the distance of. - /// the HostDistance to host. + /// when is a + /// zero-token node; for hosts + /// in the local DC; for all other hosts. public HostDistance Distance(Host host) { + if (host.IsZeroTokenNode) + { + return HostDistance.Ignored; + } + var dc = GetDatacenter(host); if (dc == _localDc) { @@ -193,10 +201,11 @@ public HostDistance Distance(Host host) /// /// Returns the hosts to use for a new query.

The returned plan will always - /// try each known host in the local datacenter first, and then, if none of the - /// local host is reachable, will try up to a configurable number of other host - /// per remote datacenter. The order of the local node in the returned query plan - /// will follow a Round-robin algorithm.

+ /// try each known routable (non-zero-token) host in the local datacenter + /// first, and then, if none of the local hosts is reachable, will try up to a + /// configurable number of other routable hosts per remote datacenter. Zero-token + /// nodes are excluded from the plan entirely. The order of the + /// local nodes in the returned query plan follows a Round-robin algorithm.

///
/// Keyspace on which the query is going to be executed /// the query for which to build the plan. @@ -223,10 +232,38 @@ public IEnumerable NewQueryPlan(string keyspace, IStatement query, bo var hosts = GetHosts(); var localHosts = hosts.Item1; var remoteHosts = hosts.Item2; - //Round-robin through local nodes - for (var i = 0; i < localHosts.Count; i++) + //Filter out zero-token nodes before applying the round-robin modulo so that the rotation is + //balanced across the routable hosts. Rotating over the full list (including skipped hosts) + //would make hosts that follow a zero-token node receive more first attempts. The filtering is + //done per query plan, so hosts that later gain tokens are observed again automatically. + //Single-pass back-fill: no allocation in the common case (no zero-token local hosts); + //one O(N) pass and one allocation in the zero-token case. + IList routableLocalHosts = localHosts; + List filteredLocalHosts = null; + for (var j = 0; j < localHosts.Count; j++) + { + var h = localHosts[j]; + if (h.IsZeroTokenNode) + { + if (filteredLocalHosts == null) + { + // First zero-token node found; back-fill all routable hosts seen so far. + filteredLocalHosts = new List(localHosts.Count); + for (var k = 0; k < j; k++) filteredLocalHosts.Add(localHosts[k]); + } + // Skip this zero-token node. + } + else + { + filteredLocalHosts?.Add(h); + } + } + if (filteredLocalHosts != null) routableLocalHosts = filteredLocalHosts; + //Round-robin through the routable local nodes + for (var i = 0; i < routableLocalHosts.Count; i++) { - yield return new HostShard(localHosts[(startIndex + i) % localHosts.Count], -1); + var host = routableLocalHosts[(startIndex + i) % routableLocalHosts.Count]; + yield return new HostShard(host, -1); } if (_usedHostsPerRemoteDc == 0) @@ -236,6 +273,10 @@ public IEnumerable NewQueryPlan(string keyspace, IStatement query, bo var dcHosts = new Dictionary(); foreach (var h in remoteHosts) { + if (h.IsZeroTokenNode) + { + continue; + } var dc = GetDatacenter(h); dcHosts.TryGetValue(dc, out int hostYieldedByDc); if (hostYieldedByDc >= _usedHostsPerRemoteDc) diff --git a/src/Cassandra/Policies/DefaultLoadBalancingPolicy.cs b/src/Cassandra/Policies/DefaultLoadBalancingPolicy.cs index 016188ecf..ed19b7ade 100644 --- a/src/Cassandra/Policies/DefaultLoadBalancingPolicy.cs +++ b/src/Cassandra/Policies/DefaultLoadBalancingPolicy.cs @@ -65,6 +65,11 @@ public DefaultLoadBalancingPolicy(string localDc) /// the HostDistance to host. public HostDistance Distance(Host host) { + if (host.IsZeroTokenNode) + { + return HostDistance.Ignored; + } + var lastPreferredHost = _lastPreferredHost; if (lastPreferredHost != null && host == lastPreferredHost) { @@ -117,7 +122,11 @@ public IEnumerable NewQueryPlan(string keyspace, IStatement statement private IEnumerable YieldPreferred(string keyspace, TargettedSimpleStatement statement) { - yield return new HostShard(statement.PreferredHost, -1); + // Skip a zero-token preferred host; it is not routable. + if (!statement.PreferredHost.IsZeroTokenNode) + { + yield return new HostShard(statement.PreferredHost, -1); + } foreach (var h in ChildPolicy.NewQueryPlan(keyspace, statement)) { yield return h; diff --git a/src/Cassandra/Policies/RoundRobinPolicy.cs b/src/Cassandra/Policies/RoundRobinPolicy.cs index 4c3ccf2b6..a33e1e3fc 100644 --- a/src/Cassandra/Policies/RoundRobinPolicy.cs +++ b/src/Cassandra/Policies/RoundRobinPolicy.cs @@ -24,13 +24,15 @@ namespace Cassandra /// /// A Round-robin load balancing policy. /// This policy queries nodes in a - /// round-robin fashion. For a given query, if an host fail, the next one - /// (following the round-robin order) is tried, until all hosts have been tried. + /// round-robin fashion. For a given query, if a host fails, the next one + /// (following the round-robin order) is tried, until all routable hosts have been tried. /// /// This policy is not datacenter aware and will include every known - /// Cassandra host in its round robin algorithm. If you use multiple datacenter - /// this will be inefficient and you will want to use the - /// load balancing policy instead. + /// routable (non-zero-token) Cassandra host in its round-robin algorithm. + /// Zero-token nodes are excluded: they are assigned + /// and never appear in a query plan. + /// If you use multiple datacenters this will be inefficient and you will want + /// to use the load balancing policy instead. /// /// public class RoundRobinPolicy : IExtendedLoadBalancingPolicy @@ -50,17 +52,23 @@ public void Initialize(ICluster cluster) /// DCAwareRoundRobinPolicy instead.

///
/// the host of which to return the distance of. - /// the HostDistance to host. + /// if is a zero-token + /// node; otherwise . public HostDistance Distance(Host host) { + if (host.IsZeroTokenNode) + { + return HostDistance.Ignored; + } + return HostDistance.Local; } /// - /// Returns the hosts to use for a new query.

The returned plan will try each - /// known host of the cluster. Upon each call to this method, the ith host of the - /// plans returned will cycle over all the host of the cluster in a round-robin - /// fashion.

+ /// Returns the hosts to use for a new query. The returned plan will try each + /// known routable host of the cluster (zero-token nodes are excluded). + /// Upon each call to this method, the ith host of the plans returned will cycle + /// over all routable hosts in a round-robin fashion. ///
/// Keyspace on which the query is going to be executed /// the query for which to build the plan. @@ -74,8 +82,32 @@ public IEnumerable NewQueryPlan(string keyspace, IStatement query) /// public IEnumerable NewQueryPlan(string keyspace, IStatement query, bool routeAsLwt) { - //shallow copy the all hosts - var hosts = (from h in _cluster.AllHosts() select h).ToArray(); + // Snapshot AllHosts() once. AllHosts() returns a live ConcurrentDictionary.Values view; + // snapshotting ensures both the zero-token check and the filter operate on the same set. + // In the common case (no zero-token nodes) the snapshot is used directly. Only when a + // zero-token node is found is a second, smaller array allocated (single-pass back-fill). + var allHosts = _cluster.AllHosts().ToArray(); + Host[] hosts = allHosts; + List filteredHosts = null; + for (var j = 0; j < allHosts.Length; j++) + { + var h = allHosts[j]; + if (h.IsZeroTokenNode) + { + if (filteredHosts == null) + { + // First zero-token node found; back-fill all routable hosts seen so far. + filteredHosts = new List(allHosts.Length); + for (var k = 0; k < j; k++) filteredHosts.Add(allHosts[k]); + } + // Skip this zero-token node. + } + else + { + filteredHosts?.Add(h); + } + } + if (filteredHosts != null) hosts = filteredHosts.ToArray(); var startIndex = 0; if (!routeAsLwt) {