Skip to content

fix: Calc Engine Concurrent Upsert Race and Orchestrator OOM Deadlocks#148

Merged
ilramdhan merged 2 commits into
mutugading:mainfrom
ilramdhan:fix/master-batch-costing
Jul 15, 2026
Merged

fix: Calc Engine Concurrent Upsert Race and Orchestrator OOM Deadlocks#148
ilramdhan merged 2 commits into
mutugading:mainfrom
ilramdhan:fix/master-batch-costing

Conversation

@ilramdhan

Copy link
Copy Markdown
Member

Description

This PR introduces critical resilience and fault-tolerance mechanisms to the distributed Calculation Engine.

First, it resolves a database transaction race condition (uk_cpc_active unique constraint violation) that occurs when two parallel calculation jobs attempt to upsert the same product cost record simultaneously. Second, it implements a self-healing "sweeper" mechanism in the Job Orchestrator to prevent the entire calculation job from stalling indefinitely if a worker pod silently crashes (e.g., due to an Out-Of-Memory kill) mid-chunk.

Type of Change

  • 🐛 Bug fix (non-breaking change that fixes an issue)
  • ✨ New feature (non-breaking change that adds functionality)
  • 💥 Breaking change (fix or feature that changes existing API)
  • ♻️ Refactor (code change without new feature or bug fix)
  • 📚 Documentation update
  • 🧪 Test update
  • 🔧 Chore (dependencies, config, etc.)

Service(s) Affected

  • Finance Service (Calc Engine / Orchestrator)
  • IAM Service
  • Shared Proto (gen/)
  • Root/Common

Changes Made

🛡️ Database Transaction Resilience

  • Concurrent Upsert Retry Loop: Added a max-3 retry loop inside UpsertWithSupersede. If two concurrent transactions race to insert a new cost record and one hits the uk_cpc_active unique constraint violation (SQLSTATE 23505), the loser catches the error, rolls back, and retries in a fresh transaction where it can properly see and supersede the winner's newly inserted row.
  • Observability: Added a new Prometheus counter finance_cost_upsert_retry_total to monitor how often this race condition is encountered and successfully mitigated in production.

⚙️ Orchestrator Fault Tolerance

  • Stuck Chunk Sweeper: Added a background sweeper goroutine to the Orchestrator (configurable, runs every 2 minutes by default).
  • Deadlock Recovery: The sweeper detects any DISPATCHED chunks that are older than 10 minutes (indicating the assigned worker likely crashed via OOM and failed to emit a ChunkCompletedEvent). It automatically marks these stuck chunks as FAILED and publishes a synthetic ChunkCompletedEvent, allowing the orchestrator to successfully advance past the stuck wave instead of hanging forever.

Related Issues

Fixes #
Related to #

API Changes (if applicable)

Proto Changes

// No Proto Changes

Breaking Changes

None.

Testing Performed

Unit Tests

  • Existing unit tests pass
  • Coverage maintained/improved

Integration Tests

  • New integration tests added
  • Existing integration tests pass

Manual Testing

# Executed heavy concurrent calculation jobs locally to intentionally trigger the uk_cpc_active race and verified the retry loop recovers successfully.
# Simulated a worker OOM by manually killing a worker process mid-chunk, and verified the orchestrator sweeper detects it after 10m and advances the job.

Lint & Build

  • golangci-lint run ./... passes
  • go build ./... succeeds
  • go test -race ./... passes

Database (if applicable)

  • Migration added
  • Migration tested (up and down)
  • No breaking schema changes (or documented)

Documentation

  • README.md updated (if needed)
  • RULES.md updated (if needed)
  • Proto comments updated
  • OpenAPI regenerated

Rollback Plan

Revert the finance-service and finance-worker deployments to the previous stable tag. No database schema rollback is required.

Screenshots/Logs (if applicable)


Pre-merge Checklist

  • I have read and followed RULES.md
  • I have read and followed CONTRIBUTING.md
  • Clean Architecture principles followed
  • All errors are properly handled
  • Context is passed appropriately
  • Structured logging is used
  • No hardcoded secrets
  • PR description is complete and clear
  • CI checks are passing

Reviewer Notes

  • Sweeper Thresholds: The sweeper runs every 2m and checks for chunks older than 10m. These thresholds were chosen assuming a single chunk should never legitimately take 10 minutes to process. If our chunk sizes or DB latency ever cause valid calculations to exceed 10 minutes, this threshold will need to be increased via environment variables.
  • Prometheus Metric: DevOps should configure an alert on finance_cost_upsert_retry_total if the rate spikes unusually high, as this could indicate severe DB contention requiring a reduction in worker concurrency.

ilramdhan and others added 2 commits July 15, 2026 21:57
…sede

When two calc jobs process the same product+period+calc_type concurrently
and no prior active row exists, both transactions see no row to supersede
and both attempt to INSERT — the second hits the uk_cpc_active unique
constraint. Wrap the supersede+insert in a retry loop (max 3 attempts)
that catches the 23505 violation, rolls back, and re-runs in a fresh
transaction where the winner's row is now visible and can be properly
superseded.

Add finance_cost_upsert_retry_total Prometheus counter for observability.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… crashes

When a worker pod crashes (OOM) mid-chunk, it never publishes a
ChunkCompletedEvent. The orchestrator's advanceAfterChunk sees
completed < total and waits forever, stalling the entire job.

Add a sweeper goroutine (every 2m, configurable) that detects DISPATCHED
chunks older than 10m, marks them FAILED, and publishes a synthetic
ChunkCompletedEvent so the orchestrator can advance past stuck waves.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@ilramdhan ilramdhan added this to the Costing Release Milestone milestone Jul 15, 2026
@ilramdhan ilramdhan self-assigned this Jul 15, 2026
Copilot AI review requested due to automatic review settings July 15, 2026 15:21
@ilramdhan ilramdhan added bug Something isn't working enhancement New feature or request fix labels Jul 15, 2026

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot was unable to review this pull request because the user who requested the review has reached their quota limit.

@ilramdhan
ilramdhan merged commit c307148 into mutugading:main Jul 15, 2026
13 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request fix

Projects

Status: Done

Development

Successfully merging this pull request may close these issues.

2 participants