diff --git a/pkgs/sdk/server/src/Internal/DataSources/FileDataSource.cs b/pkgs/sdk/server/src/Internal/DataSources/FileDataSource.cs index 91cb989b..1990622b 100644 --- a/pkgs/sdk/server/src/Internal/DataSources/FileDataSource.cs +++ b/pkgs/sdk/server/src/Internal/DataSources/FileDataSource.cs @@ -25,9 +25,15 @@ internal sealed class FileDataSource : IDataSource private readonly Logger _logger; private volatile bool _started; private volatile bool _loadedValidData; + private volatile bool _disposed; private volatile int _lastVersion; private object _updateLock = new object(); + private const int MaxRetries = 5; + private static readonly TimeSpan RetryDelay = TimeSpan.FromMilliseconds(600); + // Per-path JSON-parse retry counters. Only touched inside _updateLock. + private readonly Dictionary _retryCounts = new Dictionary(); + public FileDataSource(IDataSourceUpdates dataSourceUpdates, FileDataTypes.IFileReader fileReader, List paths, bool autoUpdate, Func alternateParser, bool skipMissingPaths, FileDataTypes.DuplicateKeysHandling duplicateKeysHandling, @@ -83,6 +89,7 @@ private void Dispose(bool disposing) { if (disposing) { + _disposed = true; _reloader?.Dispose(); } } @@ -91,6 +98,10 @@ private void LoadAll() { lock (_updateLock) { + if (_disposed) + { + return; + } var version = Interlocked.Increment(ref _lastVersion); var flags = new Dictionary(); var segments = new Dictionary(); @@ -102,11 +113,43 @@ private void LoadAll() _logger.Debug("file data: {0}", content); var data = _parser.Parse(content, version); _dataMerger.AddToData(data, flags, segments); + _retryCounts.Remove(path); } catch (FileNotFoundException) when (_skipMissingPaths) { _logger.Debug("{0}: {1}", path, "File not found"); } + catch (System.Text.Json.JsonException) + { + // A file-change notification can fire while the file is mid-write, so we may read an + // empty or partially written file. Retry up to MaxRetries times before giving up; the + // counter is cleared on the next successful load. + if (!_retryCounts.ContainsKey(path)) + { + _retryCounts[path] = 0; + } + _retryCounts[path]++; + + if (_retryCounts[path] < MaxRetries) + { + _logger.Warn("{0}: Failed to parse file, retrying in {1} ms", path, RetryDelay.TotalMilliseconds); + Task.Run(async () => + { + await Task.Delay(RetryDelay).ConfigureAwait(false); + if (_disposed) + { + return; + } + LoadAll(); + }); + } + else + { + _logger.Error("{0}: Failed to parse file after {1} retries", path, MaxRetries); + } + + return; + } catch (Exception e) { LogHelpers.LogException(_logger, "Failed to load " + path, e); diff --git a/pkgs/sdk/server/test/Internal/DataSources/FileDataSourceTest.cs b/pkgs/sdk/server/test/Internal/DataSources/FileDataSourceTest.cs index 5322a38c..d3f8a701 100644 --- a/pkgs/sdk/server/test/Internal/DataSources/FileDataSourceTest.cs +++ b/pkgs/sdk/server/test/Internal/DataSources/FileDataSourceTest.cs @@ -151,50 +151,7 @@ public void ModifiedFileIsReloadedIfAutoUpdateIsOn() file.SetContentFromPath(TestUtils.TestFilePath("segment-only.json")); - AssertHelpers.ExpectPredicate(_updateSink.Inits, actual => - { - var segments = actual.Data.First(item => item.Key == DataModel.Segments); - var features = actual.Data.First(item => item.Key == DataModel.Features); - if (!features.Value.Items.IsNullOrEmpty()) - { - return false; - } - - var segmentItems = segments.Value.Items.ToList(); - - if (segmentItems.Count != 1) - { - return false; - } - - var segmentDescriptor = segmentItems[0]; - if (segmentDescriptor.Key != "seg1") - { - return false; - } - - if (segmentDescriptor.Value.Version == 1) - { - return false; - } - - if (!(segmentDescriptor.Value.Item is Segment segment)) - { - return false; - } - - if (segment.Deleted) - { - return false; - } - - if (segment.Included.Count != 1) - { - return false; - } - - return segment.Included[0] == "user1"; - }, + AssertHelpers.ExpectPredicate(_updateSink.Inits, IsSegmentOnlyDataAfterReload, "Did not receive expected update from the file data source.", TimeSpan.FromSeconds(30)); } @@ -270,16 +227,9 @@ public void ModifiedFileIsReloadedEvenIfOneFileIsMissingIfSkipMissingPathsIsSet( file1.SetContentFromPath(TestUtils.TestFilePath("segment-only.json")); - // Use ExpectJsonValue to handle potential race conditions where the file watcher - // may trigger multiple reload events. This keeps checking events until one matches - // the expected JSON or the timeout expires. - var newData = AssertHelpers.ExpectJsonValue( - _updateSink.Inits, - DataSetAsJson(ExpectedDataSetForSegmentOnlyFile(2)), - DataSetAsJson, + AssertHelpers.ExpectPredicate(_updateSink.Inits, IsSegmentOnlyDataAfterReload, + "Did not receive expected update from the file data source.", TimeSpan.FromSeconds(30)); - - AssertJsonEqual(DataSetAsJson(ExpectedDataSetForSegmentOnlyFile(2)), DataSetAsJson(newData)); } } } @@ -298,18 +248,9 @@ public void IfFlagsAreBadAtStartTimeAutoUpdateCanStillLoadGoodDataLater() file.SetContentFromPath(TestUtils.TestFilePath("segment-only.json")); - // Use ExpectJsonValue to handle potential race conditions where the file watcher - // may trigger multiple reload events. This keeps checking events until one matches - // the expected JSON or the timeout expires. - // Note that the expected version is 2 because we increment the version on each - // *attempt* to load the files, not on each successful load. - var newData = AssertHelpers.ExpectJsonValue( - _updateSink.Inits, - DataSetAsJson(ExpectedDataSetForSegmentOnlyFile(2)), - DataSetAsJson, + AssertHelpers.ExpectPredicate(_updateSink.Inits, IsSegmentOnlyDataAfterReload, + "Did not receive expected update from the file data source.", TimeSpan.FromSeconds(30)); - - AssertJsonEqual(DataSetAsJson(ExpectedDataSetForSegmentOnlyFile(2)), DataSetAsJson(newData)); } } } @@ -365,5 +306,38 @@ private static FullDataSet ExpectedDataSetForSegmentOnlyFile(int new SegmentBuilder("seg1").Version(version).Included("user1").Build() ) .Build(); + + // Predicate that matches the structure of segment-only.json reloaded after the initial load. + // We deliberately don't pin the exact version: with the file watcher firing on truncate-then-write + // and the JsonException retry, the version of the successful load is non-deterministic, so we + // only require that it isn't the initial version 1. + private static bool IsSegmentOnlyDataAfterReload(FullDataSet actual) + { + var features = actual.Data.First(item => item.Key == DataModel.Features); + if (!features.Value.Items.IsNullOrEmpty()) + { + return false; + } + + var segments = actual.Data.First(item => item.Key == DataModel.Segments); + var segmentItems = segments.Value.Items.ToList(); + if (segmentItems.Count != 1) + { + return false; + } + + var segmentDescriptor = segmentItems[0]; + if (segmentDescriptor.Key != "seg1" || segmentDescriptor.Value.Version == 1) + { + return false; + } + + if (!(segmentDescriptor.Value.Item is Segment segment) || segment.Deleted) + { + return false; + } + + return segment.Included.Count == 1 && segment.Included[0] == "user1"; + } } }