grpc / grpc/grpc-dotnet

SubchannelsLoadBalancer.UpdateChannelState() with the same Addresses but different BalancerAddress.Attributes doesn't trigger UpdateBalancingState

Open
#2,552 2 comments 0 reactions 0 assignees View on GitHub
bug
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

Open the contributing 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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.