SubchannelsLoadBalancer.UpdateChannelState() with the same Addresses but different BalancerAddress.Attributes doesn't trigger UpdateBalancingState
- Dominant language
- C#
- Stars
- 4.5k
- Forks
- 836
- Avg merge
- 6d 3h
- Merged PRs (30d)
- 7
Description
Hi, @JamesNK
It seems that there is an issue in `SubchannelsLoadBalancer.UpdateChannelState()`.
When I call `SubchannelsLoadBalancer.UpdateChannelState(state)` with the `state` which consists of the same addresses but with different attributes as in previous call then `UpdateChannelState` does not update subchannels at all.
### Reproduction
1. Create `N` addresses with some attributes:
```
// create 2 addresses with some attributes
var address1 = new BalancerAddress(host1, port);
address1.Attributes.TryAdd(attributeKey, 20); // <-- attribute
var address2 = new BalancerAddress(host2, port);
address2.Attributes.TryAdd(attributeKey, 80); // <-- attribute
```
2. Create state based on these addresses:
```
var state1 = new ChannelState(
status: new Status(),
addresses: [address1, address2],
loadBalancingConfig: null,
attributes: new BalancerAttributes());
```
3. Update channel state:
```
balancer.UpdateChannelState(state1);
```
4. Create another `N` addresses with the same hosts and ports but another attributes:
```
var address3 = new BalancerAddress(host1, port);
address3.Attributes.TryAdd(attributeKey, 40); // <-- difference
var address4 = new BalancerAddress(host2, port);
address4.Attributes.TryAdd(attributeKey, 60); // <-- difference
```
5. Create new state based on new addresses:
```
var state2 = new ChannelState(
status: new Status(),
addresses: [address3, address4],
loadBalancingConfig: null,
attributes: new BalancerAttributes());
```
5. Update channel state:
```
balancer.UpdateChannelState(state2);
```
6. Make sure that channels left unchanged
### Where is the problem?
I believe that the problem is in channel updates check code:
```
public override void UpdateChannelState(ChannelState state)
{
...
var allUpdatedSubchannels = new List();
var newSubchannels = new List();
var currentSubchannels = _addressSubchannels.ToList();
foreach (var address in state.Addresses)
{
var i = FindSubchannelByAddress(currentSubchannels, address); // <-- Found because addresses are the same
AddressSubchannel newOrCurrentSubchannel;
if (i != null) // <-- Not null because addresses are the same
{
// <-- Step into this block because addresses are the same
newOrCurrentSubchannel = currentSubchannels[i.Value];
currentSubchannels.RemoveAt(i.Value); // <-- Always remove
// <-- True because attributes are different
if (!BalancerAddressEqualityComparer.Instance.Equals(address, newOrCurrentSubchannel.Address))
{
// <-- Step into this block because attributes are different
newOrCurrentSubchannel = new AddressSubchannel(
newOrCurrentSubchannel.Subchannel,
address,
newOrCurrentSubchannel.LastKnownState);
newOrCurrentSubchannel.Subchannel.UpdateAddresses(new[] { address });
}
...
}
else
{
// <-- Never step into this block
...
}
allUpdatedSubchannels.Add(newOrCurrentSubchannel);
}
var removedSubConnections = currentSubchannels; // <-- Empty because addresses are the same
// <-- `newSubchannels` is empty because we newer touched it in the code above
if (removedSubConnections.Count == 0 && newSubchannels.Count == 0)
{
// <-- Always step here because `removedSubConnections` and `newSubchannels` are empty
// <-- Do nothing except some logging
...
return; // <-- Always return here
}
....
// <-- Newer came here
UpdateBalancingState(state.Status);
```
I created simple test in your repository fork to shot the problem: https://github.com/kolonist/grpc-dotnet/blob/SubchannelsLoadBalancerTests/test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs
Main test is the first - `UpdateChannelState_Should_Update_channel_state_with_different_attributes_only`.
Another two tests show that different count of addresses leads to real update of balancing state.
### System info
**What version of gRPC and what language are you using?** - All versions, including `2.66.x`
**What operating system (Linux, Windows,...) and version?** - All versions. Tested on MacOS arm64 and Linux intel x64
**What runtime / compiler are you using (e.g. .NET Core SDK version `dotnet --info`)**
```
SDK:
Version: 8.0.401
Commit: 811edcc344
Workload version: 8.0.400-manifests.57f7c351
MSBuild version: 17.11.4+37eb419ad
Environment:
OS Name: Mac OS X
OS Version: 14.4
OS Platform: Darwin
RID: osx-arm64
Base Path: /usr/local/share/dotnet/sdk/8.0.401/
Host:
Version: 8.0.8
Architecture: arm64
.NET SDKs installed:
8.0.401 [/usr/local/share/dotnet/sdk]
.NET runtimes installed:
Microsoft.NETCore.App 8.0.8 [/usr/local/share/dotnet/shared/Microsoft.NETCore.App]
Other architectures found:
x64 [/usr/local/share/dotnet/x64]
registered at [/etc/dotnet/install_location_x64]
Environment variables:
Not set
global.json file:
Not found
```
Contributor guide
Research direction
Start in SubchannelsLoadBalancer.UpdateChannelState() and review the update and early-return logic shown in the issue. Run test/Grpc.Net.Client.Tests/Infrastructure/Balancer/SubchannelsLoadBalancerTests.cs, especially UpdateChannelState_Should_Update_channel_state_with_different_attributes_only. Done means differing BalancerAddress.Attributes trigger the expected subchannel and balancing-state update without changing the address count.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- csharp
- Domain
- networking
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100