From 8736ee52280193bc7fd4919b8e361b256dbf4ea3 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 8 Oct 2024 20:42:10 +0800 Subject: [PATCH] Add fix and update tests --- global.json | 3 +- .../Balancer/SubchannelsLoadBalancer.cs | 5 +- .../Balancer/SubchannelsLoadBalancerTests.cs | 199 ++++++++++++ .../Balancer/SubchannelsLoadBalancerTests.cs | 292 ------------------ 4 files changed, 204 insertions(+), 295 deletions(-) create mode 100644 test/Grpc.Net.Client.Tests/Balancer/SubchannelsLoadBalancerTests.cs delete mode 100644 test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs diff --git a/global.json b/global.json index 67d91a4f9..0519736ab 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,5 @@ { "sdk": { - "version": "9.0.100-preview.7.24407.12", - "rollForward": "latestFeature" + "version": "9.0.100-rc.1.24452.12" } } diff --git a/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs b/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs index 5f27c347b..b9b9b2ee3 100644 --- a/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs +++ b/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs @@ -122,6 +122,7 @@ public override void UpdateChannelState(ChannelState state) var allUpdatedSubchannels = new List(); var newSubchannels = new List(); + var modifiedSubchannelsCount = 0; var currentSubchannels = _addressSubchannels.ToList(); // The state's addresses is the new authoritative list of addresses. @@ -150,6 +151,8 @@ public override void UpdateChannelState(ChannelState state) address, newOrCurrentSubchannel.LastKnownState); newOrCurrentSubchannel.Subchannel.UpdateAddresses(new[] { address }); + + modifiedSubchannelsCount++; } SubchannelLog.SubchannelPreserved(_logger, newOrCurrentSubchannel.Subchannel.Id, address); @@ -171,7 +174,7 @@ public override void UpdateChannelState(ChannelState state) // This can all be removed. var removedSubConnections = currentSubchannels; - if (removedSubConnections.Count == 0 && newSubchannels.Count == 0) + if (removedSubConnections.Count == 0 && newSubchannels.Count == 0 && modifiedSubchannelsCount == 0) { SubchannelsLoadBalancerLog.ConnectionsUnchanged(_logger); return; diff --git a/test/Grpc.Net.Client.Tests/Balancer/SubchannelsLoadBalancerTests.cs b/test/Grpc.Net.Client.Tests/Balancer/SubchannelsLoadBalancerTests.cs new file mode 100644 index 000000000..533ce0e3f --- /dev/null +++ b/test/Grpc.Net.Client.Tests/Balancer/SubchannelsLoadBalancerTests.cs @@ -0,0 +1,199 @@ +#region Copyright notice and license + +// Copyright 2019 The gRPC Authors +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#endregion + +#if SUPPORT_LOAD_BALANCING +using System.Net; +using Grpc.Core; +using Grpc.Net.Client.Balancer; +using Grpc.Net.Client.Balancer.Internal; +using Grpc.Tests.Shared; +using Microsoft.Extensions.Logging; +using Microsoft.Extensions.Logging.Abstractions; + +namespace Grpc.Net.Client.Tests.Infrastructure.Balancer; + +[TestFixture] +public class SubchannelsLoadBalancerTests +{ + [Test] + public void UpdateChannelState_AddressMatchAndAttributesDifferent_UpdateState() + { + // Arrange + const string host1 = "127.0.0.1"; + const string host2 = "127.0.0.2"; + const int port = 80; + + const string attributeKey = "key1"; + + var controller = new CustomChannelControlHelper(); + var balancer = new CustomBalancer(controller, NullLoggerFactory.Instance); + + // create 2 addresses with some attributes + var address1 = new BalancerAddress(host1, port); + address1.Attributes.TryAdd(attributeKey, 20); // <-- difference + + var address2 = new BalancerAddress(host2, port); + address2.Attributes.TryAdd(attributeKey, 80); // <-- difference + + var state1 = new ChannelState( + status: new Status(), + addresses: [address1, address2], + loadBalancingConfig: null, + attributes: new BalancerAttributes()); + + // create 2 addresses with the same hosts and ports as previous but with other attribute values + var address3 = new BalancerAddress(host1, port); + address3.Attributes.TryAdd(attributeKey, 40); // <-- difference + + var address4 = new BalancerAddress(host2, port); + address4.Attributes.TryAdd(attributeKey, 60); // <-- difference + + var state2 = new ChannelState( + status: new Status(), + addresses: [address3, address4], + loadBalancingConfig: null, + attributes: new BalancerAttributes()); + + // Act + // first update with `address1` and `address2` + balancer.UpdateChannelState(state1); + + // remember count of `IChannelControlHelper.UpdateState()` calls + var updateStateCallsCount1 = controller.UpdateStateCallsCount; + + // second update with `address3` and `address4` + // which differs from `address1` and `address2` _only_ in attributes values + balancer.UpdateChannelState(state2); + + // get count of `IChannelControlHelper.UpdateState()` calls after second update + var updateStateCallsCount2 = controller.UpdateStateCallsCount; + + // Assert + Assert.True( + updateStateCallsCount2 > updateStateCallsCount1, + "`IChannelControlHelper.UpdateState()` was not called from `SubchannelsLoadBalancer.UpdateChannelState()`"); + } +} + +file class CustomBalancer( + IChannelControlHelper controller, + ILoggerFactory loggerFactory) + : SubchannelsLoadBalancer(controller, loggerFactory) +{ + protected override SubchannelPicker CreatePicker(IReadOnlyList readySubchannels) + { + return new CustomPicker(readySubchannels); + } +} + +file class CustomPicker : SubchannelPicker +{ + private IReadOnlyList readySubchannels; + + public CustomPicker(IReadOnlyList readySubchannels) + { + this.readySubchannels = readySubchannels; + } + + public override PickResult Pick(PickContext context) + { + return PickResult.ForSubchannel(readySubchannels[0]); + } +} + +file class CustomChannelControlHelper : IChannelControlHelper +{ + public int UpdateStateCallsCount { get; private set; } + + public Subchannel CreateSubchannel(SubchannelOptions options) + { + var subchannelTransportFactory = new CustomSubchannelTransportFactory(); + + var manager = new ConnectionManager( + new CustomResolver(), + true, + NullLoggerFactory.Instance, + new CustomBackoffPolicyFactory(), + subchannelTransportFactory, + []); + + return ((IChannelControlHelper)manager).CreateSubchannel(options); + } + + public void UpdateState(BalancerState state) + { + UpdateStateCallsCount++; + } + + public void RefreshResolver() { } +} + +file class CustomResolver() : PollingResolver(NullLoggerFactory.Instance) +{ + protected override Task ResolveAsync(CancellationToken cancellationToken) + { + return Task.CompletedTask; + } +} + +file class CustomBackoffPolicyFactory : IBackoffPolicyFactory +{ + public IBackoffPolicy Create() + { + return new CustomBackoffPolicy(); + } +} + +file class CustomBackoffPolicy : IBackoffPolicy +{ + public TimeSpan NextBackoff() + { + return TimeSpan.Zero; + } +} + +file class CustomSubchannelTransportFactory : ISubchannelTransportFactory +{ + public ISubchannelTransport Create(Subchannel subchannel) + { + return new CustomSubchannelTransport(); + } +} + +file class CustomSubchannelTransport : ISubchannelTransport +{ + public void Dispose() { } + + public DnsEndPoint? CurrentEndPoint { get; } + public TimeSpan? ConnectTimeout { get; } + public TransportStatus TransportStatus { get; } + + public ValueTask GetStreamAsync(DnsEndPoint endPoint, CancellationToken cancellationToken) + { + return ValueTask.FromResult(new MemoryStream()); + } + + public ValueTask TryConnectAsync(ConnectContext context, int attempt) + { + return ValueTask.FromResult(ConnectResult.Success); + } + + public void Disconnect() { } +} + +#endif diff --git a/test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs b/test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs deleted file mode 100644 index af04dc575..000000000 --- a/test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs +++ /dev/null @@ -1,292 +0,0 @@ -#if SUPPORT_LOAD_BALANCING - -using System.Net; -using Grpc.Core; -using Grpc.Net.Client.Balancer; -using Grpc.Net.Client.Balancer.Internal; -using Grpc.Tests.Shared; -using Microsoft.Extensions.Logging; -using Microsoft.Extensions.Logging.Abstractions; -using Xunit; - -namespace Grpc.Net.Client.Tests.Infrastructure.Balancer; - -public class SubchannelsLoadBalancerTests -{ - [Fact] - public void UpdateChannelState_Should_Update_channel_state_with_different_attributes_only() - { - /* Arrange */ - - const string host1 = "127.0.0.1"; - const string host2 = "127.0.0.2"; - const int port = 80; - - const string attributeKey = "key1"; - - var controller = new CustomChannelControlHelper(); - var balancer = new CustomBalancer(controller, NullLoggerFactory.Instance); - - // create 2 addresses with some attributes - var address1 = new BalancerAddress(host1, port); - address1.Attributes.TryAdd(attributeKey, 20); // <-- difference - - var address2 = new BalancerAddress(host2, port); - address2.Attributes.TryAdd(attributeKey, 80); // <-- difference - - var state1 = new ChannelState( - status: new Status(), - addresses: [address1, address2], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - // create 2 addresses with the same hosts and ports as previous but with other attribute values - var address3 = new BalancerAddress(host1, port); - address3.Attributes.TryAdd(attributeKey, 40); // <-- difference - - var address4 = new BalancerAddress(host2, port); - address4.Attributes.TryAdd(attributeKey, 60); // <-- difference - - var state2 = new ChannelState( - status: new Status(), - addresses: [address3, address4], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - /* Act */ - - // first update with `address1` and `address2` - balancer.UpdateChannelState(state1); - - // remember count of `IChannelControlHelper.UpdateState()` calls - var updateStateCallsCount1 = controller.UpdateStateCallsCount; - - // second update with `address3` and `address4` - // which differs from `address1` and `address2` _only_ in attributes values - balancer.UpdateChannelState(state2); - - // get count of `IChannelControlHelper.UpdateState()` calls after second update - var updateStateCallsCount2 = controller.UpdateStateCallsCount; - - Assert.True( - updateStateCallsCount2 > updateStateCallsCount1, - "`IChannelControlHelper.UpdateState()` was not called from `SubchannelsLoadBalancer.UpdateChannelState()`"); - } - - [Fact] - public void UpdateChannelState_Should_Update_channel_state_with_shorter_address_list() - { - /* Arrange */ - - const string host1 = "127.0.0.1"; - const string host2 = "127.0.0.2"; - const int port = 80; - - const string attributeKey = "key1"; - - var controller = new CustomChannelControlHelper(); - var balancer = new CustomBalancer(controller, NullLoggerFactory.Instance); - - // create 2 addresses with some attributes - var address1 = new BalancerAddress(host1, port); - address1.Attributes.TryAdd(attributeKey, 20); // <-- difference - - var address2 = new BalancerAddress(host2, port); - address2.Attributes.TryAdd(attributeKey, 80); // <-- difference - - var state1 = new ChannelState( - status: new Status(), - addresses: [address1, address2], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - // create 1 address with the same host and port as one of the previous addresses but with other attribute value - var address3 = new BalancerAddress(host1, port); - address3.Attributes.TryAdd(attributeKey, 40); // <-- difference - - var state2 = new ChannelState( - status: new Status(), - addresses: [address3], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - /* Act */ - - // first update with `address1` and `address2` - balancer.UpdateChannelState(state1); - - // remember count of `IChannelControlHelper.UpdateState()` calls - var updateStateCallsCount1 = controller.UpdateStateCallsCount; - - // second update with `address3` and `address4` - // which differs from `address1` and `address2` _only_ in attributes values - balancer.UpdateChannelState(state2); - - // get count of `IChannelControlHelper.UpdateState()` calls after second update - var updateStateCallsCount2 = controller.UpdateStateCallsCount; - - Assert.True( - updateStateCallsCount2 > updateStateCallsCount1, - "`IChannelControlHelper.UpdateState()` was not called from `SubchannelsLoadBalancer.UpdateChannelState()`"); - } - - [Fact] - public void UpdateChannelState_Should_Update_channel_state_with_longer_address_list() - { - /* Arrange */ - - const string host1 = "127.0.0.1"; - const string host2 = "127.0.0.2"; - const int port = 80; - - const string attributeKey = "key1"; - const string attributeKey2 = "key2"; - - var controller = new CustomChannelControlHelper(); - var balancer = new CustomBalancer(controller, NullLoggerFactory.Instance); - - // create 2 addresses with some attributes - var address1 = new BalancerAddress(host1, port); - address1.Attributes.TryAdd(attributeKey, 20); // <-- difference - - var address2 = new BalancerAddress(host2, port); - address2.Attributes.TryAdd(attributeKey, 80); // <-- difference - - var state1 = new ChannelState( - status: new Status(), - addresses: [address1, address2], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - // create 2 addresses with the same hosts and ports as previous but with other attribute values - var address3 = new BalancerAddress(host1, port); - address3.Attributes.TryAdd(attributeKey, 40); // <-- difference - - var address4 = new BalancerAddress(host2, port); - address4.Attributes.TryAdd(attributeKey, 60); // <-- difference - - // create one extra address with another fake attribute - var address5 = new BalancerAddress(host2, port); - address5.Attributes.TryAdd(attributeKey, 60); - address5.Attributes.TryAdd(attributeKey2, "Fake"); // <-- difference - - var state2 = new ChannelState( - status: new Status(), - addresses: [address3, address4, address5], - loadBalancingConfig: null, - attributes: new BalancerAttributes()); - - /* Act */ - - // first update with `address1` and `address2` - balancer.UpdateChannelState(state1); - - // remember count of `IChannelControlHelper.UpdateState()` calls - var updateStateCallsCount1 = controller.UpdateStateCallsCount; - - // second update with `address3` and `address4` - // which differs from `address1` and `address2` _only_ in attributes values - balancer.UpdateChannelState(state2); - - // get count of `IChannelControlHelper.UpdateState()` calls after second update - var updateStateCallsCount2 = controller.UpdateStateCallsCount; - - Assert.True( - updateStateCallsCount2 > updateStateCallsCount1, - "`IChannelControlHelper.UpdateState()` was not called from `SubchannelsLoadBalancer.UpdateChannelState()`"); - } -} - -/* Helper chasses */ -file class CustomBalancer( - IChannelControlHelper controller, - ILoggerFactory loggerFactory) - : SubchannelsLoadBalancer(controller, loggerFactory) -{ - protected override SubchannelPicker CreatePicker(IReadOnlyList readySubchannels) - { - return new CustomPicker(readySubchannels); - } -} - -file class CustomChannelControlHelper : IChannelControlHelper -{ - public int UpdateStateCallsCount { get; private set; } - - public Subchannel CreateSubchannel(SubchannelOptions options) - { - var subchannelTransportFactory = new CustomSubchannelTransportFactory(); - - var manager = new ConnectionManager( - new CustomResolver(), - true, - NullLoggerFactory.Instance, - new CustomBackoffPolicyFactory(), - subchannelTransportFactory, - []); - - return ((IChannelControlHelper)manager).CreateSubchannel(options); - } - - public void UpdateState(BalancerState state) - { - UpdateStateCallsCount++; - } - - public void RefreshResolver() { } -} - -file class CustomResolver() : PollingResolver(NullLoggerFactory.Instance) -{ - protected override Task ResolveAsync(CancellationToken cancellationToken) - { - return Task.CompletedTask; - } -} - -file class CustomBackoffPolicyFactory : IBackoffPolicyFactory -{ - public IBackoffPolicy Create() - { - return new CustomBackoffPolicy(); - } -} - -file class CustomBackoffPolicy : IBackoffPolicy -{ - public TimeSpan NextBackoff() - { - return TimeSpan.Zero; - } -} - -file class CustomSubchannelTransportFactory: ISubchannelTransportFactory -{ - public ISubchannelTransport Create(Subchannel subchannel) - { - return new CustomSubchannelTransport(); - } -} - -file class CustomSubchannelTransport : ISubchannelTransport -{ - public void Dispose() { } - - public DnsEndPoint? CurrentEndPoint { get; } - public TimeSpan? ConnectTimeout { get; } - public TransportStatus TransportStatus { get; } - - public ValueTask GetStreamAsync(DnsEndPoint endPoint, CancellationToken cancellationToken) - { - return ValueTask.FromResult(new MemoryStream()); - } - - public ValueTask TryConnectAsync(ConnectContext context, int attempt) - { - return ValueTask.FromResult(ConnectResult.Success); - } - - public void Disconnect() { } -} - -#endif