From 06408f5ab2daa30c75c9e7408b339d8e5a5cb887 Mon Sep 17 00:00:00 2001 From: adams85 <31276480+adams85@users.noreply.github.com> Date: Fri, 6 Sep 2024 16:35:30 +0200 Subject: [PATCH] Fix cache expiration-related issues in Auto Polling mode (#98) * Check for expiration on every iteration in Auto Polling mode + allow a tolerance of 500ms to prevent missing fetches due to timer inaccuracy + sync with cache even in offline mode * Fix broken badge in README.md * Minor correction to README.md * Bump version --- README.md | 5 +- appveyor.yml | 2 +- .../ConfigServiceTests.cs | 103 +++++++++++++++++- .../ConfigService/AutoPollConfigService.cs | 38 +++---- 4 files changed, 120 insertions(+), 28 deletions(-) diff --git a/README.md b/README.md index 95a7d474..e78cabb6 100644 --- a/README.md +++ b/README.md @@ -8,7 +8,8 @@ ConfigCat is a feature flag and configuration management service that lets you s ConfigCat is a [hosted feature flag service](https://configcat.com). Manage feature toggles across frontend, backend, mobile, desktop apps. [Alternative to LaunchDarkly](https://configcat.com). Management app + feature flag SDKs. -[![Build status](https://ci.appveyor.com/api/projects/status/3kygp783vc2uv9xr?svg=true)](https://ci.appveyor.com/project/ConfigCat/net-sdk) [![NuGet Version](https://buildstats.info/nuget/ConfigCat.Client)](https://www.nuget.org/packages/ConfigCat.Client/) +[![Build status](https://ci.appveyor.com/api/projects/status/3kygp783vc2uv9xr?svg=true)](https://ci.appveyor.com/project/ConfigCat/net-sdk) +[![NuGet Version](https://img.shields.io/nuget/v/ConfigCat.Client)](https://www.nuget.org/packages/ConfigCat.Client/) [![Sonar Coverage](https://img.shields.io/sonar/coverage/net-sdk?logo=SonarCloud&server=https%3A%2F%2Fsonarcloud.io)](https://sonarcloud.io/project/overview?id=net-sdk) [![Quality Gate Status](https://sonarcloud.io/api/project_badges/measure?project=net-sdk&metric=alert_status)](https://sonarcloud.io/dashboard?id=net-sdk) [![License: MIT](https://img.shields.io/badge/License-MIT-yellow.svg)](https://github.com/configcat/.net-sdk/blob/master/LICENSE) @@ -106,7 +107,7 @@ Based on our tests, the SDK is compatible with the following runtimes/deployment *Unity WebGL also works but needs a bit of extra effort: you will need to enable WebGL compatibility by calling the `ConfigCatClient.PlatformCompatibilityOptions.EnableUnityWebGLCompatibility` method. For more details, see [Sample Scripts](https://github.com/configcat/.net-sdk/tree/master/samples/UnityWebGL).
**To make the SDK work in Release builds on UWP, you will need to add `` to your application's [.rd.xml](https://learn.microsoft.com/en-us/windows/uwp/dotnet-native/runtime-directives-rd-xml-configuration-file-reference) file. See also [this discussion](https://github.com/dotnet/runtime/issues/29912#issuecomment-638471351). -We strive to provide an extensive support for the various .NET runtimes and versions. If you still encounter an issue with the SDK on some platform, please open a [GitHub issue](https://github.com/configcat/.net-sdk/issues/new/choose) or contact support. +> We strive to provide an extensive support for the various .NET runtimes and versions. If you still encounter an issue with the SDK on some platform, please open a [GitHub issue](https://github.com/configcat/.net-sdk/issues/new/choose) or contact support. ## Need help? https://configcat.com/support diff --git a/appveyor.yml b/appveyor.yml index 12cd852c..aa08bb3c 100644 --- a/appveyor.yml +++ b/appveyor.yml @@ -1,5 +1,5 @@ environment: - build_version: 9.3.0 + build_version: 9.3.1 version: $(build_version)-{build} image: Visual Studio 2022 configuration: Release diff --git a/src/ConfigCat.Client.Tests/ConfigServiceTests.cs b/src/ConfigCat.Client.Tests/ConfigServiceTests.cs index f9d39139..d00a1184 100644 --- a/src/ConfigCat.Client.Tests/ConfigServiceTests.cs +++ b/src/ConfigCat.Client.Tests/ConfigServiceTests.cs @@ -1,5 +1,6 @@ using System; using System.Collections.Concurrent; +using System.Diagnostics; using System.Threading; using System.Threading.Tasks; using ConfigCat.Client.Cache; @@ -13,11 +14,19 @@ namespace ConfigCat.Client.Tests; [TestClass] public class ConfigServiceTests { - private static ProjectConfig CreateExpiredPc(DateTime timeStamp, TimeSpan expiration, string configJson = "{}", string httpETag = "\"67890\"") => - ConfigHelper.FromString(configJson, httpETag, timeStamp - expiration - TimeSpan.FromSeconds(1)); + private static ProjectConfig CreateExpiredPc(DateTime timeStamp, TimeSpan expiration, string configJson = "{}", string httpETag = "\"67890\"") + { + var offset = TimeSpan.FromSeconds(1); + Debug.Assert(offset.TotalMilliseconds > AutoPollConfigService.PollExpirationToleranceMs * 1.5); + return ConfigHelper.FromString(configJson, httpETag, timeStamp - expiration - offset); + } - private static ProjectConfig CreateUpToDatePc(DateTime timeStamp, TimeSpan expiration, string configJson = "{}", string httpETag = "\"abcdef\"") => - ConfigHelper.FromString(configJson, httpETag, timeStamp - expiration + TimeSpan.FromSeconds(1)); + private static ProjectConfig CreateUpToDatePc(DateTime timeStamp, TimeSpan expiration, string configJson = "{}", string httpETag = "\"abcdef\"") + { + var offset = TimeSpan.FromSeconds(1); + Debug.Assert(offset.TotalMilliseconds > AutoPollConfigService.PollExpirationToleranceMs * 1.5); + return ConfigHelper.FromString(configJson, httpETag, timeStamp - expiration + offset); + } private static ProjectConfig CreateFreshPc(DateTime timeStamp, string configJson = "{}", string httpETag = "\"12345\"") => ConfigHelper.FromString(configJson, httpETag, timeStamp); @@ -300,6 +309,92 @@ public async Task AutoPollConfigService_GetConfigAsync_WithTimer_ShouldInvokeFet this.fetcherMock.Verify(m => m.FetchAsync(cachedPc, It.IsAny()), Times.Once); } + [DataTestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task AutoPollConfigService_GetConfig_ShouldReturnCachedConfigWhenCachedConfigIsNotExpired(bool isAsync) + { + // Arrange + + var pollInterval = TimeSpan.FromSeconds(2); + + var timeStamp = ProjectConfig.GenerateTimeStamp(); + var fetchedPc = CreateFreshPc(timeStamp); + var cachedPc = fetchedPc.With(fetchedPc.TimeStamp - pollInterval + TimeSpan.FromMilliseconds(1.5 * AutoPollConfigService.PollExpirationToleranceMs)); + + const string cacheKey = ""; + var cache = new InMemoryConfigCache(); + cache.Set(cacheKey, cachedPc); + + this.fetcherMock + .Setup(m => m.FetchAsync(cachedPc, It.IsAny())) + .ReturnsAsync(FetchResult.Success(fetchedPc)); + + var config = PollingModes.AutoPoll(pollInterval, maxInitWaitTime: Timeout.InfiniteTimeSpan); + using var service = new AutoPollConfigService(config, + this.fetcherMock.Object, + new CacheParameters(cache, cacheKey), + this.loggerMock.Object.AsWrapper(), + startTimer: true); + + // Act + + // Give a bit of time to the polling loop to do the first iteration. + await Task.Delay(TimeSpan.FromTicks(pollInterval.Ticks / 4)); + + var actualPc = isAsync ? await service.GetConfigAsync() : service.GetConfig(); + + // Assert + + Assert.AreSame(cachedPc, actualPc); + + this.fetcherMock.Verify(m => m.FetchAsync(cachedPc, It.IsAny()), Times.Never); + } + + [DataTestMethod] + [DataRow(false)] + [DataRow(true)] + public async Task AutoPollConfigService_GetConfig_ShouldWaitForFetchWhenCachedConfigIsExpired(bool isAsync) + { + // Arrange + + var pollInterval = TimeSpan.FromSeconds(2); + + var timeStamp = ProjectConfig.GenerateTimeStamp(); + var fetchedPc = CreateFreshPc(timeStamp); + var cachedPc = fetchedPc.With(fetchedPc.TimeStamp - pollInterval + TimeSpan.FromMilliseconds(0.5 * AutoPollConfigService.PollExpirationToleranceMs)); + + const string cacheKey = ""; + var cache = new InMemoryConfigCache(); + cache.Set(cacheKey, cachedPc); + + this.fetcherMock + .Setup(m => m.FetchAsync(cachedPc, It.IsAny())) + .ReturnsAsync(FetchResult.Success(fetchedPc)); + + var config = PollingModes.AutoPoll(pollInterval, maxInitWaitTime: Timeout.InfiniteTimeSpan); + using var service = new AutoPollConfigService(config, + this.fetcherMock.Object, + new CacheParameters(cache, cacheKey), + this.loggerMock.Object.AsWrapper(), + startTimer: true); + + // Act + + // Give a bit of time to the polling loop to do the first iteration. + await Task.Delay(TimeSpan.FromTicks(pollInterval.Ticks / 4)); + + var actualPc = isAsync ? await service.GetConfigAsync() : service.GetConfig(); + + // Assert + + Assert.AreNotSame(cachedPc, actualPc); + Assert.AreEqual(cachedPc.HttpETag, actualPc.HttpETag); + Assert.AreEqual(cachedPc.ConfigJson, actualPc.ConfigJson); + + this.fetcherMock.Verify(m => m.FetchAsync(cachedPc, It.IsAny()), Times.Once); + } + [TestMethod] public async Task AutoPollConfigService_RefreshConfigAsync_ShouldOnceInvokeCacheGetAndFetchAndCacheSet() { diff --git a/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs b/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs index 0cdb4ac8..a42e78da 100644 --- a/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs +++ b/src/ConfigCatClient/ConfigService/AutoPollConfigService.cs @@ -9,7 +9,10 @@ namespace ConfigCat.Client.ConfigService; internal sealed class AutoPollConfigService : ConfigServiceBase, IConfigService { + internal const int PollExpirationToleranceMs = 500; + private readonly TimeSpan pollInterval; + private readonly TimeSpan pollExpiration; private readonly TimeSpan maxInitWaitTime; private readonly CancellationTokenSource initSignalCancellationTokenSource = new(); // used for signalling initialization ready private CancellationTokenSource timerCancellationTokenSource = new(); // used for signalling background work to stop @@ -34,6 +37,10 @@ internal AutoPollConfigService( SafeHooksWrapper hooks = default) : base(configFetcher, cacheParameters, logger, isOffline, hooks) { this.pollInterval = options.PollInterval; + // Due to the inaccuracy of the timer, some tolerance should be allowed when checking for + // cache expiration in the polling loop, otherwise some fetch operations may be missed. + this.pollExpiration = options.PollInterval - TimeSpan.FromMilliseconds(PollExpirationToleranceMs); + this.maxInitWaitTime = options.MaxInitWaitTime >= TimeSpan.Zero ? options.MaxInitWaitTime : Timeout.InfiniteTimeSpan; var initialCacheSyncUpTask = SyncUpWithCacheAsync(WaitForReadyCancellationToken); @@ -214,34 +221,23 @@ private void StartScheduler(Task? initialCacheSyncUpTask, Cancell }, stopToken); } - private async ValueTask PollCoreAsync(bool isFirstIteration, Task? initialCacheSyncUpTask, CancellationToken cancellationToken) + private async ValueTask PollCoreAsync(bool isFirstIteration, Task? initialCacheSyncUpTask, CancellationToken stopToken) { - if (isFirstIteration) - { - var latestConfig = initialCacheSyncUpTask is not null - ? await initialCacheSyncUpTask.WaitAsync(cancellationToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext) - : await this.ConfigCache.GetAsync(base.CacheKey, cancellationToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); + var latestConfig = initialCacheSyncUpTask is not null + ? await initialCacheSyncUpTask.WaitAsync(stopToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext) + : await this.ConfigCache.GetAsync(base.CacheKey, stopToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); - if (latestConfig.IsExpired(expiration: this.pollInterval)) - { - if (!IsOffline) - { - await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: false, cancellationToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); - } - } - else - { - SignalInitialization(); - } - } - else + if (latestConfig.IsExpired(expiration: this.pollExpiration)) { if (!IsOffline) { - var latestConfig = await this.ConfigCache.GetAsync(base.CacheKey, cancellationToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); - await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: false, cancellationToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); + await RefreshConfigCoreAsync(latestConfig, isInitiatedByUser: false, stopToken).ConfigureAwait(TaskShim.ContinueOnCapturedContext); } } + else if (isFirstIteration) + { + SignalInitialization(); + } } public override ClientCacheState GetCacheState(ProjectConfig cachedConfig)