Skip to content

fix: config traversal, timer rethrow, stack-trace reset, lock scope (MAPCO-11323) - #258

Open
shimoncohen wants to merge 1 commit into
masterfrom
logic-4-correctness
Open

fix: config traversal, timer rethrow, stack-trace reset, lock scope (MAPCO-11323)#258
shimoncohen wants to merge 1 commit into
masterfrom
logic-4-correctness

Conversation

@shimoncohen

Copy link
Copy Markdown
Collaborator

LOGIC-4 of MAPCO-11317. Four independent correctness fixes.

Fixes

  1. Config traversalConfigurationManager.GetChildren walked from the root section on every loop iteration (not the accumulated section) and its bounds re-applied the final key. Works for 2-part paths — which is the only current caller, GetChildren("TASK","types") — but silently wrong for 1-part and 3+-part paths. Now traverses the accumulated section.
  2. Timer rethrowHeartbeatClient.Send is a Timer.Elapsed handler that rethrew on failure. System.Timers.Timer discards it, and an unhandled exception on the timer thread can tear down the process. Now logs and lets the next tick retry.
  3. Stack-trace resetS3Client used throw e; on its non-key error paths, resetting the exception's stack trace. Changed to throw;.
  4. Lock scopeGpkg/Fs/S3 held their batch _locker as static, so every data-source instance serialized paging against every other instance. The guarded state is per-instance, so the lock is now per-instance. Also unblocks parallelism for SVC-1 (MAPCO-11325).

Testing

Full suite: 1141 passed, 0 failed. Fixes 1 and 4 preserve current behavior for existing call sites (2-part config path; single-instance paging) while correcting the latent/ cross-instance cases; 2 and 3 change only failure-path behavior. ConfigurationManager builds its IConfiguration from files in its ctor and isn't injectable, so GetChildren has no isolated unit test; the fix is exercised via the TaskRunner path.

🤖 Generated with Claude Code

…MAPCO-11323)

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) <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants