Skip to content

fix(metrics): _Accumulator.observe() has no lock, causing lost increments under concurrent async tasks #253

Closed

Description

@piyushmakhija28

Summary

_Accumulator.observe() in metrics.py performs a read-modify-write on plain Python int/float fields with no synchronisation. While ProductMetrics.record_routing() acquires the outer _lock, the observe() call within is not atomic under asyncio + thread-pool workers — two concurrent observers can read the same counter value, both increment it, and write back the same result, silently losing one observation.

Root Cause

backend/app/productization/metrics.py around line 38:

class _Accumulator:
    count: int = 0
    total: float = 0.0

    def observe(self, value: float) -> None:
        self.count += 1       # ← not atomic
        self.total += value   # ← not atomic

ProductMetrics._lock guards the record_routing() method body, but if multiple tasks call record_routing() truly concurrently from different thread-pool slots, and CPython's GIL is released between the LOAD_FAST and STORE_FAST bytecodes for self.count += 1, a race occurs. Under high concurrency with uvicorn workers the effect compounds.

Failure Scenario

  1. 1,000 concurrent routing requests all call record_routing().
  2. Two workers simultaneously read self.routing_count=500, both write 501.
  3. One increment is silently lost. Repeated thousands of times.
  4. /metrics reports rag_routing_total=850 instead of 1000.
  5. fallback_rate = fallback_count / routing_count → wrong denominator → incorrect PRD KPI dashboard.

Proposed Fix

Either use the existing outer lock correctly (the lock should be held during observe):

def record_routing(self, ...) -> None:
    with self._lock:
        self.routing_latency_ms.observe(latency_ms)   # observe inside lock
        self.routing_count += 1
        if fallback:
            self.fallback_count += 1

Or use threading.Lock inside _Accumulator:

@dataclass
class _Accumulator:
    _lock: threading.Lock = field(default_factory=threading.Lock)
    count: int = 0
    total: float = 0.0

    def observe(self, value: float) -> None:
        with self._lock:
            self.count += 1
            self.total += value

Activity

  1. added
    bugSomething isn't working
    priority: highData loss, security, or major correctness bug
    code-reviewFound via automated code review
    on Jun 21, 2026
  2. piyushmakhija28 commented on Jun 21, 2026

    @piyushmakhija28
    ContributorAuthor

    Fixed in PR #280 (Sprint R3/R4: fix(sprint-r3): root-cause fixes for all 39 bugs from code review round 3 + Sprint R4 round 4)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    bugSomething isn't workingcode-reviewFound via automated code reviewpriority: highData loss, security, or major correctness bug

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions