From 0f2ffdc03edcd832e6f83a9deb73e2d57c0b8dbf Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Wed, 9 Oct 2024 10:01:33 +0800 Subject: [PATCH] Fix UpdateBalancingState not called when address attributes are modified (#2553) --- examples/Container/Backend/Dockerfile | 4 +- examples/Container/Frontend/Dockerfile | 4 +- global.json | 2 +- .../Balancer/SubchannelsLoadBalancer.cs | 5 +- .../Balancer/SubchannelsLoadBalancerTests.cs | 199 ++++++++++++++++++ .../InteropTestsGrpcWebWebsite/Dockerfile | 4 +- testassets/InteropTestsWebsite/Dockerfile | 4 +- 7 files changed, 212 insertions(+), 10 deletions(-) create mode 100644 test/Grpc.Net.Client.Tests/Balancer/SubchannelsLoadBalancerTests.cs diff --git a/examples/Container/Backend/Dockerfile b/examples/Container/Backend/Dockerfile index 78b8007d5..7c0429870 100644 --- a/examples/Container/Backend/Dockerfile +++ b/examples/Container/Backend/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet/nightly/sdk:9.0-preview AS build-env +FROM mcr.microsoft.com/dotnet/sdk:9.0 AS build-env WORKDIR /app # Copy everything @@ -8,7 +8,7 @@ RUN dotnet restore examples/Container/Backend RUN dotnet publish examples/Container/Backend -c Release -o out # Build runtime image -FROM mcr.microsoft.com/dotnet/nightly/aspnet:9.0-preview +FROM mcr.microsoft.com/dotnet/aspnet:9.0 WORKDIR /app COPY --from=build-env /app/out . ENTRYPOINT ["dotnet", "Backend.dll"] \ No newline at end of file diff --git a/examples/Container/Frontend/Dockerfile b/examples/Container/Frontend/Dockerfile index 23403110c..017712b23 100644 --- a/examples/Container/Frontend/Dockerfile +++ b/examples/Container/Frontend/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet/nightly/sdk:9.0-preview AS build-env +FROM mcr.microsoft.com/dotnet/sdk:9.0 AS build-env WORKDIR /app # Copy everything @@ -8,7 +8,7 @@ RUN dotnet restore examples/Container/Frontend RUN dotnet publish examples/Container/Frontend -c Release -o out # Build runtime image -FROM mcr.microsoft.com/dotnet/nightly/aspnet:9.0-preview +FROM mcr.microsoft.com/dotnet/aspnet:9.0 WORKDIR /app COPY --from=build-env /app/out . ENTRYPOINT ["dotnet", "Frontend.dll"] \ No newline at end of file diff --git a/global.json b/global.json index 67d91a4f9..0344c423c 100644 --- a/global.json +++ b/global.json @@ -1,6 +1,6 @@ { "sdk": { - "version": "9.0.100-preview.7.24407.12", + "version": "9.0.100-rc.2.24474.11", "rollForward": "latestFeature" } } diff --git a/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs b/src/Grpc.Net.Client/Balancer/SubchannelsLoadBalancer.cs index 5f27c347b..73b88f7e1 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 hasModifiedSubchannels = false; 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 }); + + hasModifiedSubchannels = true; } 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 && !hasModifiedSubchannels) { 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/testassets/InteropTestsGrpcWebWebsite/Dockerfile b/testassets/InteropTestsGrpcWebWebsite/Dockerfile index c8ecbf5c2..9a71faacf 100644 --- a/testassets/InteropTestsGrpcWebWebsite/Dockerfile +++ b/testassets/InteropTestsGrpcWebWebsite/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet/nightly/sdk:9.0-preview AS build-env +FROM mcr.microsoft.com/dotnet/sdk:9.0 AS build-env WORKDIR /app # Copy everything @@ -8,7 +8,7 @@ RUN dotnet restore testassets/InteropTestsGrpcWebWebsite RUN dotnet publish testassets/InteropTestsGrpcWebWebsite -c Release -o out # Build runtime image -FROM mcr.microsoft.com/dotnet/nightly/aspnet:9.0-preview +FROM mcr.microsoft.com/dotnet/aspnet:9.0 WORKDIR /app COPY --from=build-env /app/out . ENTRYPOINT ["dotnet", "InteropTestsGrpcWebWebsite.dll", "--urls", "http://+:80"] diff --git a/testassets/InteropTestsWebsite/Dockerfile b/testassets/InteropTestsWebsite/Dockerfile index 16e916257..59cdeb214 100644 --- a/testassets/InteropTestsWebsite/Dockerfile +++ b/testassets/InteropTestsWebsite/Dockerfile @@ -1,4 +1,4 @@ -FROM mcr.microsoft.com/dotnet/nightly/sdk:9.0-preview AS build-env +FROM mcr.microsoft.com/dotnet/sdk:9.0 AS build-env WORKDIR /app # Copy everything @@ -8,7 +8,7 @@ RUN dotnet restore testassets/InteropTestsWebsite RUN dotnet publish testassets/InteropTestsWebsite --framework net9.0 -c Release -o out -p:LatestFramework=true # Build runtime image -FROM mcr.microsoft.com/dotnet/nightly/aspnet:9.0-preview +FROM mcr.microsoft.com/dotnet/aspnet:9.0 WORKDIR /app COPY --from=build-env /app/out . ENTRYPOINT ["dotnet", "InteropTestsWebsite.dll", "--port_http1", "80"] \ No newline at end of file