From 8dee97253a8d92c55a77f6336644736d0a6162e3 Mon Sep 17 00:00:00 2001 From: shimoncohen Date: Tue, 4 Aug 2026 10:35:17 +0300 Subject: [PATCH 1/2] fix: config traversal, timer rethrow, stack-trace reset, lock scope (MAPCO-11323) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four independent correctness fixes in MergerLogic. - ConfigurationManager.GetChildren: the traversal loop read from the root config each iteration instead of the accumulated section, and its bounds re-applied the last key. Correct for 2-part paths (the only current caller) but wrong for 1- and 3+-part paths. Walk the accumulated section instead. - HeartbeatClient.Send (Timer.Elapsed handler): rethrew on failure. The Timer discards it and an unhandled exception on the timer thread can tear down the process. Log and let the next tick retry. - S3Client: `throw e;` reset the exception stack trace on the non-key error paths; use `throw;` to preserve it. - Gpkg/Fs/S3: the batch _locker was static, so every instance serialized its paging against every other instance. Make it per-instance — the state it guards is per-instance — which also unblocks parallelism for SVC-1. Co-Authored-By: Claude Opus 4.8 (1M context) --- MergerLogic/Clients/HeartbeatClient.cs | 3 ++- MergerLogic/Clients/S3Client.cs | 4 ++-- MergerLogic/DataTypes/Fs.cs | 2 +- MergerLogic/DataTypes/Gpkg.cs | 2 +- MergerLogic/DataTypes/S3.cs | 2 +- MergerLogic/Utils/ConfigurationManager.cs | 9 +++++---- 6 files changed, 12 insertions(+), 10 deletions(-) diff --git a/MergerLogic/Clients/HeartbeatClient.cs b/MergerLogic/Clients/HeartbeatClient.cs index 4160e4e6..076b4bee 100644 --- a/MergerLogic/Clients/HeartbeatClient.cs +++ b/MergerLogic/Clients/HeartbeatClient.cs @@ -84,8 +84,9 @@ public void Send(object? sender, ElapsedEventArgs elapsedEventArgs) } catch (Exception e) { + // Elapsed runs on a timer thread; the Timer discards anything thrown here and an + // unhandled exception can tear down the process. Log and let the next tick retry. this._logger.LogError($"[{MethodBase.GetCurrentMethod().Name}] Could not send heartbeat for task={this._taskId}, {e.Message}"); - throw; } } } diff --git a/MergerLogic/Clients/S3Client.cs b/MergerLogic/Clients/S3Client.cs index f06c0e23..78ca6097 100644 --- a/MergerLogic/Clients/S3Client.cs +++ b/MergerLogic/Clients/S3Client.cs @@ -75,7 +75,7 @@ private bool IsKeyError(Exception e) return null; } // In case there are other errors such as connection to S3 - throw e; + throw; } } @@ -169,7 +169,7 @@ public void UpdateTile(Tile tile) return null; } // In case there are other errors such as connection to S3 - throw e; + throw; } } } diff --git a/MergerLogic/DataTypes/Fs.cs b/MergerLogic/DataTypes/Fs.cs index 3fd4ca50..73e168bc 100644 --- a/MergerLogic/DataTypes/Fs.cs +++ b/MergerLogic/DataTypes/Fs.cs @@ -16,7 +16,7 @@ public class FS : Data private IFileSystem _fileSystem; private readonly string[] _supportedFileExtensions = { ".png", ".jpg", ".jpeg" }; - static readonly object _locker = new object(); + private readonly object _locker = new object(); public FS(IPathUtils pathUtils, IServiceProvider container, string path, int batchSize, Grid? grid, GridOrigin? origin, bool isBase = false) : base(container, DataType.FOLDER, path, batchSize, grid, origin, isBase) diff --git a/MergerLogic/DataTypes/Gpkg.cs b/MergerLogic/DataTypes/Gpkg.cs index 9b1af8cf..d70f2a3a 100644 --- a/MergerLogic/DataTypes/Gpkg.cs +++ b/MergerLogic/DataTypes/Gpkg.cs @@ -11,7 +11,7 @@ public class Gpkg : Data private long _offset; private Extent _extent; private readonly IConfigurationManager _configManager; - static readonly object _locker = new object(); + private readonly object _locker = new object(); public Gpkg(IConfigurationManager configuration, IServiceProvider container, string path, int batchSize, Grid? grid, GridOrigin? origin, bool isBase = false, Extent? extent = null) diff --git a/MergerLogic/DataTypes/S3.cs b/MergerLogic/DataTypes/S3.cs index 84cdd79e..dd555740 100644 --- a/MergerLogic/DataTypes/S3.cs +++ b/MergerLogic/DataTypes/S3.cs @@ -17,7 +17,7 @@ public class S3 : Data private IEnumerator _zoomEnumerator; private string? _continuationToken; private bool _endOfRead; - static readonly object _locker = new object(); + private readonly object _locker = new object(); private const string nullStringValue = "Null"; private readonly IPathUtils _pathUtils; diff --git a/MergerLogic/Utils/ConfigurationManager.cs b/MergerLogic/Utils/ConfigurationManager.cs index 983ee679..0f4ce2d8 100644 --- a/MergerLogic/Utils/ConfigurationManager.cs +++ b/MergerLogic/Utils/ConfigurationManager.cs @@ -25,12 +25,13 @@ public ConfigurationManager(ILogger? logger) public IEnumerable GetChildren(params string[] configPath) { - var config = this.config.GetSection(configPath[0]); - for (int i = 1; i < configPath.Length - 1; i++) + var section = this.config.GetSection(configPath[0]); + for (int i = 1; i < configPath.Length; i++) { - config = this.config.GetSection(configPath[i]); + // Traverse into the accumulated section, not back to the root each iteration. + section = section.GetSection(configPath[i]); } - return config.GetSection(configPath[configPath.Length - 1]).GetChildren(); + return section.GetChildren(); } public string GetConfiguration(params string[] configPath) From 7ea1c55959bda55217e3237f1fd7ed4b58273097 Mon Sep 17 00:00:00 2001 From: shimoncohen Date: Thu, 6 Aug 2026 17:53:19 +0300 Subject: [PATCH 2/2] refactor: simplify GetChildren to colon-joined key lookup Collapse the manual section-traversal loop to a single colon-joined GetSection call, matching GetConfiguration's style. IConfiguration treats ":" as the hierarchy separator, so this nests through all segments and is equivalent for the sole caller (length-2 "TASK:types") while also handling paths of any length. Co-Authored-By: Claude Opus 4.8 (1M context) --- MergerLogic/Utils/ConfigurationManager.cs | 9 ++------- 1 file changed, 2 insertions(+), 7 deletions(-) diff --git a/MergerLogic/Utils/ConfigurationManager.cs b/MergerLogic/Utils/ConfigurationManager.cs index 0f4ce2d8..1952014f 100644 --- a/MergerLogic/Utils/ConfigurationManager.cs +++ b/MergerLogic/Utils/ConfigurationManager.cs @@ -25,13 +25,8 @@ public ConfigurationManager(ILogger? logger) public IEnumerable GetChildren(params string[] configPath) { - var section = this.config.GetSection(configPath[0]); - for (int i = 1; i < configPath.Length; i++) - { - // Traverse into the accumulated section, not back to the root each iteration. - section = section.GetSection(configPath[i]); - } - return section.GetChildren(); + string key = string.Join(":", configPath); + return this.config.GetSection(key).GetChildren(); } public string GetConfiguration(params string[] configPath)