From 8ab493e9f114ddf2c4b56806ee12c57132310e7c Mon Sep 17 00:00:00 2001 From: Jose Date: Fri, 24 Apr 2026 11:40:14 +0200 Subject: [PATCH 1/2] Log background cache refresh failures in LocationResolver (#4428) Co-Authored-By: Claude Opus 4.6 (1M context) --- .../Internal/LocationResolver.cs | 30 +++++++++++++++++-- src/IceRpc.Locator/LocationEventId.cs | 5 +++- src/IceRpc.Locator/LocatorInterceptor.cs | 3 +- .../LocationResolverTests.cs | 16 ++++++---- 4 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/IceRpc.Locator/Internal/LocationResolver.cs b/src/IceRpc.Locator/Internal/LocationResolver.cs index 221213e01b..98935ff0be 100644 --- a/src/IceRpc.Locator/Internal/LocationResolver.cs +++ b/src/IceRpc.Locator/Internal/LocationResolver.cs @@ -29,6 +29,17 @@ internal static partial void LogFailedToResolve( string locationKind, Location location, Exception? exception = null); + + [LoggerMessage( + EventId = (int)LocationEventId.BackgroundRefreshFailed, + EventName = nameof(LocationEventId.BackgroundRefreshFailed), + Level = LogLevel.Debug, + Message = "Background cache refresh failed for {LocationKind} '{Location}'")] + internal static partial void LogBackgroundRefreshFailed( + this ILogger logger, + string locationKind, + Location location, + Exception exception); } /// An implementation of without a cache. @@ -67,6 +78,7 @@ internal CacheLessLocationResolver(IServerAddressFinder serverAddressFinder) => internal class LocationResolver : ILocationResolver { private readonly bool _background; + private readonly ILogger _logger; private readonly IServerAddressCache _serverAddressCache; private readonly IServerAddressFinder _serverAddressFinder; private readonly TimeSpan _refreshThreshold; @@ -78,13 +90,15 @@ internal LocationResolver( IServerAddressCache serverAddressCache, bool background, TimeSpan refreshThreshold, - TimeSpan ttl) + TimeSpan ttl, + ILogger logger) { _serverAddressFinder = serverAddressFinder; _serverAddressCache = serverAddressCache; _background = background; _refreshThreshold = refreshThreshold; _ttl = ttl; + _logger = logger; } public ValueTask<(ServiceAddress? ServiceAddress, bool FromCache)> ResolveAsync( @@ -118,7 +132,7 @@ internal LocationResolver( else if (_background && expired) { // We retrieved an expired service address from the cache, so we launch a refresh in the background. - _ = _serverAddressFinder.FindAsync(location, cancellationToken: default).ConfigureAwait(false); + _ = RefreshInBackgroundAsync(location); } // A well-known service address resolution can return a service address with an adapter-id. @@ -151,6 +165,18 @@ internal LocationResolver( return (serviceAddress, serviceAddress is not null && !resolved); } + + private async Task RefreshInBackgroundAsync(Location location) + { + try + { + _ = await _serverAddressFinder.FindAsync(location, cancellationToken: default).ConfigureAwait(false); + } + catch (Exception exception) + { + _logger.LogBackgroundRefreshFailed(location.Kind, location, exception); + } + } } /// A decorator that adds event source logging to a location resolver. diff --git a/src/IceRpc.Locator/LocationEventId.cs b/src/IceRpc.Locator/LocationEventId.cs index 7909439f68..7d69e47d30 100644 --- a/src/IceRpc.Locator/LocationEventId.cs +++ b/src/IceRpc.Locator/LocationEventId.cs @@ -25,5 +25,8 @@ public enum LocationEventId FindFailed, /// The server address finder found server address(es) for the given location. - Found + Found, + + /// A background cache refresh failed. + BackgroundRefreshFailed } diff --git a/src/IceRpc.Locator/LocatorInterceptor.cs b/src/IceRpc.Locator/LocatorInterceptor.cs index ff8336d97a..5857035d8e 100644 --- a/src/IceRpc.Locator/LocatorInterceptor.cs +++ b/src/IceRpc.Locator/LocatorInterceptor.cs @@ -221,7 +221,8 @@ public LocatorLocationResolver(ILocator locator, LocatorOptions options, ILogger serverAddressCache, options.Background, options.RefreshThreshold, - options.Ttl); + options.Ttl, + logger); if (logger != NullLogger.Instance) { _locationResolver = new LogLocationResolverDecorator(_locationResolver, logger); diff --git a/tests/IceRpc.Locator.Tests/LocationResolverTests.cs b/tests/IceRpc.Locator.Tests/LocationResolverTests.cs index d2e07e15d7..7b80042940 100644 --- a/tests/IceRpc.Locator.Tests/LocationResolverTests.cs +++ b/tests/IceRpc.Locator.Tests/LocationResolverTests.cs @@ -1,6 +1,7 @@ // Copyright (c) ZeroC, Inc. using IceRpc.Locator.Internal; +using Microsoft.Extensions.Logging.Abstractions; using NUnit.Framework; namespace IceRpc.Locator.Tests; @@ -21,7 +22,8 @@ public async Task ServerAddress_finder_not_called_when_cache_entry_age_is_less_o new MockServerAddressCache(cachedServiceAddress, insertionTime: TimeSpan.FromSeconds(cacheEntryAge)), background: false, TimeSpan.FromSeconds(refreshThreshold), - ttl: Timeout.InfiniteTimeSpan); + ttl: Timeout.InfiniteTimeSpan, + NullLogger.Instance); (ServiceAddress? resolved, bool _) = await resolver.ResolveAsync(default, refreshCache: true, default); @@ -44,7 +46,8 @@ public async Task ServerAddress_finder_called_when_cache_entry_age_is_greater_th new MockServerAddressCache(cachedServiceAddress, insertionTime: TimeSpan.FromSeconds(cacheEntryAge)), background: false, TimeSpan.FromSeconds(refreshThreshold), - ttl: Timeout.InfiniteTimeSpan); + ttl: Timeout.InfiniteTimeSpan, + NullLogger.Instance); (ServiceAddress? resolved, bool _) = await resolver.ResolveAsync(default, refreshCache: true, default); @@ -64,7 +67,8 @@ public async Task ServerAddress_finder_called_on_background() new MockServerAddressCache(cachedServiceAddress, insertionTime: TimeSpan.FromSeconds(120)), background: true, TimeSpan.FromSeconds(1), - ttl: TimeSpan.FromSeconds(30)); + ttl: TimeSpan.FromSeconds(30), + NullLogger.Instance); (ServiceAddress? resolved, bool fromCache) = await resolver.ResolveAsync(default, refreshCache: false, default); @@ -84,7 +88,8 @@ public async Task Location_recursive_resolution() new MockServerAddressCache(), background: true, TimeSpan.FromSeconds(1), - ttl: TimeSpan.FromSeconds(30)); + ttl: TimeSpan.FromSeconds(30), + NullLogger.Instance); (ServiceAddress? resolved, bool fromCache) = await resolver.ResolveAsync( new Location @@ -111,7 +116,8 @@ public async Task Failure_to_recursively_resolve_adapter_id_removes_proxy_from_c serverAddressCache, background: true, TimeSpan.FromSeconds(1), - ttl: TimeSpan.FromSeconds(30)); + ttl: TimeSpan.FromSeconds(30), + NullLogger.Instance); var location = new Location { Value = "/hello", From 3c3bca675f6816eb469d5dfea0570b8a5c8e228b Mon Sep 17 00:00:00 2001 From: Jose Date: Mon, 27 Apr 2026 12:18:32 +0200 Subject: [PATCH 2/2] Convert RefreshInBackgroundAsync to a local function MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Matches the pattern used elsewhere in the repo (e.g. ClientConnection's PerformDisposeAsync/PerformShutdownAsync) — the helper is only invoked from inside PerformResolveAsync and captures the enclosing 'location' parameter, so a local function expresses that scope more directly than a private instance method. --- .../Internal/LocationResolver.cs | 20 +++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/src/IceRpc.Locator/Internal/LocationResolver.cs b/src/IceRpc.Locator/Internal/LocationResolver.cs index 98935ff0be..6947eaafae 100644 --- a/src/IceRpc.Locator/Internal/LocationResolver.cs +++ b/src/IceRpc.Locator/Internal/LocationResolver.cs @@ -132,7 +132,7 @@ internal LocationResolver( else if (_background && expired) { // We retrieved an expired service address from the cache, so we launch a refresh in the background. - _ = RefreshInBackgroundAsync(location); + _ = RefreshInBackgroundAsync(); } // A well-known service address resolution can return a service address with an adapter-id. @@ -164,17 +164,17 @@ internal LocationResolver( } return (serviceAddress, serviceAddress is not null && !resolved); - } - private async Task RefreshInBackgroundAsync(Location location) - { - try - { - _ = await _serverAddressFinder.FindAsync(location, cancellationToken: default).ConfigureAwait(false); - } - catch (Exception exception) + async Task RefreshInBackgroundAsync() { - _logger.LogBackgroundRefreshFailed(location.Kind, location, exception); + try + { + _ = await _serverAddressFinder.FindAsync(location, cancellationToken: default).ConfigureAwait(false); + } + catch (Exception exception) + { + _logger.LogBackgroundRefreshFailed(location.Kind, location, exception); + } } } }