← Back to System Internals

Reviewing Correctness in Distributed & Concurrent Code

The hardest bugs to catch in review are the ones that only appear on a retry, a race, or a partial failure.

This is the deepest technical question in the code-review track, and for an ads-events-infra engineer it's the one where you should sound most fluent. Distributed and concurrent code is correct on the happy path and subtly wrong the moment a message is redelivered, two threads interleave, a node dies mid-write, or a clock skews. None of those show up in a single-threaded unit test. The interviewer wants to hear that when you review ingestion, execution, or any multi-node path, you're running a mental model of “what happens if this runs twice, out of order, or half-fails” — and that you know the concrete failure modes by name.

What this question is really testing

Whether you have internalized the fallacies of distributed computing and the concurrency memory model well enough to spot violations in someone else's diff. The tell is that you don't assume reliable, ordered, exactly-once delivery or a single-threaded world — you assume the opposite and ask the code to prove it's safe. This is also where allocation and concurrency in core execution/ingestion paths matter: a data race or a lock held across I/O in the hot path is both a correctness bug and a performance one.

How to answer

• Idempotency. The first question for any handler or write: is it safe to run twice? Networks retry, queues redeliver, consumers restart. Look for idempotency keys, dedup, or naturally idempotent operations (upsert by key) rather than blind increments or appends.
• At-least-once vs exactly-once. Most systems give at-least-once. “Exactly-once” is usually at-least-once delivery plus idempotent processing, or transactional dedup. Be skeptical of any code that assumes a message arrives exactly one time without a dedup mechanism.
• Ordering assumptions. Does the code assume events arrive in order? Across partitions, shards, or producers, they don't. Check whether correctness depends on order and, if so, whether ordering is actually guaranteed (single partition key, sequence numbers, versioning).
• Races and thread-safety. Look for shared mutable state touched from multiple threads without synchronization — check-then-act (TOCTOU), non-atomic read-modify-write, a HashMap shared across threads. Confirm the locking discipline is real, not hopeful.
• Memory-model / visibility. A write on one thread isn't automatically visible to another. Look for missing volatile/atomics/memory barriers, and for “it works on my machine” publication of objects without a happens-before edge.
• Partial failure. What if the process dies between step 1 and step 2 of a multi-step write? Look for non-atomic sequences that leave state half-applied, and for the absence of a compensating action, saga, or transactional boundary.
• Retry storms and deadline propagation. Retries need backoff, jitter, and a bound, or they amplify an outage. Deadlines/timeouts must propagate down the call chain — a caller that already gave up shouldn't leave downstream work running.
• Consistency and clocks. Be explicit about the consistency model the code assumes (strong vs eventual) and whether that matches the store. Never use wall-clock time for ordering or correctness across nodes — clocks skew; prefer logical clocks / versions.
• Deadlocks. Multiple locks acquired in inconsistent order, or a lock held across a blocking call, is a deadlock or a latency cliff. Check lock ordering and scope.

What the interviewer is looking for

• “What if it runs twice / out of order / half-fails” as a reflex, not an afterthought.
• Idempotency named as the first defense, with a concrete mechanism.
• Fluency with the memory model — visibility, not just mutual exclusion.
• Deadline/retry hygiene tied to avoiding cascading failure.
• Concurrency treated as both a correctness and a hot-path performance concern.

The most common catch — a non-idempotent, non-atomic counter update on an at-least-once stream:

# WRONG: redelivery double-counts; read-modify-write races across consumers
def on_click(event):
    count = store.get(event.ad_id)        # two consumers read the same value
    store.put(event.ad_id, count + 1)     # lost update + double count on retry

# RIGHT: dedup by event id, then an atomic op that's safe to replay
def on_click(event):
    if not store.mark_seen(event.event_id):  # idempotency key, first-writer-wins
        return                               # duplicate delivery -> no-op
    store.atomic_incr(event.ad_id, 1)        # atomic, no lost update

Common follow-ups

Where to get your data (Meta)

• SEV / postmortems — incidents from double-processing, lost updates, retry storms, or ordering assumptions; the review guardrail each produced.
• Phabricator — a review where you caught a missing idempotency key, a data race, or a non-atomic multi-step write.
• Scuba / ODS — duplicate-rate, data-loss, and lag dashboards that reveal delivery-semantics bugs at scale.
• Internal wiki — the ingestion/consumer correctness guidelines (idempotency, dedup, ordering) your team follows.
• Design-review docs — where the consistency model and failure handling for a path were decided.

---