Skip to content

Commit

Permalink
Refactor transport to use endpoint instead of BalancerAddress in logging
Browse files Browse the repository at this point in the history
  • Loading branch information
JamesNK committed Aug 27, 2023
1 parent 6d23e73 commit db9a001
Show file tree
Hide file tree
Showing 9 changed files with 120 additions and 111 deletions.
19 changes: 18 additions & 1 deletion src/Grpc.Net.Client/Balancer/BalancerAddress.cs
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ namespace Grpc.Net.Client.Balancer;
/// </summary>
public sealed class BalancerAddress
{
// Internal so address attributes can be compared without allocating an empty collection.
internal BalancerAttributes? _attributes;

/// <summary>
Expand All @@ -48,7 +49,7 @@ public BalancerAddress(DnsEndPoint endPoint)
/// <param name="host">The host.</param>
/// <param name="port">The port.</param>
[DebuggerStepThrough]
public BalancerAddress(string host, int port) : this(new DnsEndPoint(host, port))
public BalancerAddress(string host, int port) : this(new BalancerEndPoint(host, port))
{
}

Expand All @@ -69,5 +70,21 @@ public override string ToString()
{
return $"{EndPoint.Host}:{EndPoint.Port}";
}

private sealed class BalancerEndPoint : DnsEndPoint
{
private string? _cachedToString;

public BalancerEndPoint(string host, int port) : base(host, port)
{
}

public override string ToString()
{
// Improve ToString performance when logging by caching ToString.
// Don't include DnsEndPoint address family.
return _cachedToString ??= $"{Host}:{Port}";
}
}
}
#endif
Original file line number Diff line number Diff line change
Expand Up @@ -94,7 +94,7 @@ internal async ValueTask<Stream> OnConnect(SocketsHttpConnectionContext context,
}

Debug.Assert(context.DnsEndPoint.Equals(currentAddress.EndPoint), "Context endpoint should equal address endpoint.");
return await subchannel.Transport.GetStreamAsync(currentAddress, cancellationToken).ConfigureAwait(false);
return await subchannel.Transport.GetStreamAsync(currentAddress.EndPoint, cancellationToken).ConfigureAwait(false);
}
#endif

Expand Down
4 changes: 2 additions & 2 deletions src/Grpc.Net.Client/Balancer/Internal/ISubchannelTransport.cs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
#region Copyright notice and license
#region Copyright notice and license

// Copyright 2019 The gRPC Authors
//
Expand Down Expand Up @@ -33,7 +33,7 @@ internal interface ISubchannelTransport : IDisposable
TimeSpan? ConnectTimeout { get; }
TransportStatus TransportStatus { get; }

ValueTask<Stream> GetStreamAsync(BalancerAddress address, CancellationToken cancellationToken);
ValueTask<Stream> GetStreamAsync(DnsEndPoint endPoint, CancellationToken cancellationToken);
ValueTask<ConnectResult> TryConnectAsync(ConnectContext context);

void Disconnect();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ public void Dispose()
_currentEndPoint = null;
}

public ValueTask<Stream> GetStreamAsync(BalancerAddress address, CancellationToken cancellationToken)
public ValueTask<Stream> GetStreamAsync(DnsEndPoint endPoint, CancellationToken cancellationToken)
{
throw new NotSupportedException();
}
Expand Down

Large diffs are not rendered by default.

6 changes: 3 additions & 3 deletions test/FunctionalTests/Balancer/ConnectionTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -346,7 +346,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() =>
}, "Wait for connections to start.");
foreach (var t in activeStreams)
{
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), t.Address.EndPoint);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), t.EndPoint);
}

// Act
Expand All @@ -367,7 +367,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() =>
activeStreams = transport.GetActiveStreams();
return activeStreams.Count == 11;
}, "Wait for connections to start.");
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), activeStreams[activeStreams.Count - 1].Address.EndPoint);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), activeStreams[activeStreams.Count - 1].EndPoint);

tcs.SetResult(null);

Expand Down Expand Up @@ -407,7 +407,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() =>

activeStreams = transport.GetActiveStreams();
Assert.AreEqual(1, activeStreams.Count);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50052), activeStreams[0].Address.EndPoint);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50052), activeStreams[0].EndPoint);
}

#if NET7_0_OR_GREATER
Expand Down
6 changes: 3 additions & 3 deletions test/FunctionalTests/Balancer/PickFirstBalancerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -371,7 +371,7 @@ async Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext conte
Assert.GreaterOrEqual(activeStreams.Count, 2);
foreach (var stream in activeStreams)
{
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), stream.Address.EndPoint);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50051), stream.EndPoint);
}

tcs.SetResult(null);
Expand All @@ -385,7 +385,7 @@ async Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext conte
await TestHelpers.AssertIsTrueRetryAsync(() =>
{
activeStreams = transport.GetActiveStreams();
Logger.LogInformation($"Current active stream addresses: {string.Join(", ", activeStreams.Select(s => s.Address))}");
Logger.LogInformation($"Current active stream addresses: {string.Join(", ", activeStreams.Select(s => s.EndPoint))}");
return activeStreams.Count == 0;
}, "Active streams removed.", Logger).DefaultTimeout();

Expand All @@ -395,7 +395,7 @@ await TestHelpers.AssertIsTrueRetryAsync(() =>

activeStreams = transport.GetActiveStreams();
Assert.AreEqual(1, activeStreams.Count);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50052), activeStreams[0].Address.EndPoint);
Assert.AreEqual(new DnsEndPoint("127.0.0.1", 50052), activeStreams[0].EndPoint);
}

[Test]
Expand Down
8 changes: 4 additions & 4 deletions test/FunctionalTests/Balancer/RoundRobinBalancerTests.cs
Original file line number Diff line number Diff line change
Expand Up @@ -381,8 +381,8 @@ Task<HelloReply> UnaryMethod(HelloRequest request, ServerCallContext context)

var activeStreams = ((SocketConnectivitySubchannelTransport)disposedSubchannel.Transport).GetActiveStreams();
Assert.AreEqual(1, activeStreams.Count);
Assert.AreEqual("127.0.0.1", activeStreams[0].Address.EndPoint.Host);
Assert.AreEqual(50051, activeStreams[0].Address.EndPoint.Port);
Assert.AreEqual("127.0.0.1", activeStreams[0].EndPoint.Host);
Assert.AreEqual(50051, activeStreams[0].EndPoint.Port);

// Wait until connected to new endpoint
Subchannel? newSubchannel = null;
Expand Down Expand Up @@ -415,8 +415,8 @@ await TestHelpers.AssertIsTrueRetryAsync(() =>
// New subchannel stream created with request.
activeStreams = ((SocketConnectivitySubchannelTransport)newSubchannel.Transport).GetActiveStreams();
Assert.AreEqual(1, activeStreams.Count);
Assert.AreEqual("127.0.0.1", activeStreams[0].Address.EndPoint.Host);
Assert.AreEqual(50052, activeStreams[0].Address.EndPoint.Port);
Assert.AreEqual("127.0.0.1", activeStreams[0].EndPoint.Host);
Assert.AreEqual(50052, activeStreams[0].EndPoint.Port);
Assert.IsNull(((SocketConnectivitySubchannelTransport)disposedSubchannel.Transport)._initialSocket);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void Dispose()
{
}

public ValueTask<Stream> GetStreamAsync(BalancerAddress address, CancellationToken cancellationToken)
public ValueTask<Stream> GetStreamAsync(DnsEndPoint endPoint, CancellationToken cancellationToken)
{
return new ValueTask<Stream>(new MemoryStream());
}
Expand Down

0 comments on commit db9a001

Please sign in to comment.