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.
Student edition · understand, apply, verify
Review replay, races, partial failure, and ordering at the effect's actual transactional boundary.
What you should be able to do: Prevent both lost updates and lost events; distinguish dedup safety from eventual completion.
A context-dependent practice guide, not a universal policy or verified personal history. Examples and numbers are illustrative. Use genuine, authorized evidence; personnel/privacy/legal decisions follow applicable organization policies and qualified review.
Concepts explained in this guide
Read these definitions first, or use the linked terms as you go. The examples and limitations are local to this guide; companion pages are optional.
Idempotency keys
A client-generated identity for one logical operation. The server binds it to caller scope, endpoint, request fingerprint, state, and result. A retry uses the same key and reuses the original outcome rather than repeating the effect.
Example. Payment p42 times out after the provider charged it. Retrying with p42 lets the provider return the same charge instead of creating a new one.
Boundary. A unique local claim is insufficient if the business effect and saved result can diverge. Keep unknown outcomes pending, use downstream idempotency, and state the retention/retry horizon.
Offsets and checkpoints
An offset is a position in one log partition. A checkpoint is a recoverable, consistent record of input positions and processing state. On restart, state must correspond exactly to the input that is considered processed.
Example. If a batch increments a total and crashes before saving its offset, replay increments twice. Commit the total, dedup state, and partition position together, or use a compatible transactional/idempotent sink.
Boundary. Saving an offset after a write avoids that specific loss window but can still duplicate; framework checkpoints alone do not make arbitrary external effects transactional.
Transactional outbox
A business transaction writes its state change and an event row to the same database. A relay publishes committed event rows later, retrying until the broker durably accepts them.
Example. An order and its OrderCreated outbox row commit together. A relay crash after publish but before marking sent publishes the same event ID again on restart.
Boundary. The relay is at-least-once, not magically exactly-once. Consumers dedup transactionally; preserve per-entity sequence when order matters. CDC is change data capture: reading committed database changes.
Optimistic concurrency and compare-and-set
Update state only if the expected version or status still matches, in the same atomic operation that changes it. A failed comparison means a conflicting operation won; reread or return a conflict.
Example. UPDATE seats SET status='held', version=version+1 WHERE id=? AND status='available' allows only one concurrent claimant to update the row.
Boundary. Application-level GET then unconditional SET is not equivalent. Include acquisition identity to reject stale callbacks, and couple related invariants in a transaction.
Fencing tokens
A strictly increasing grant number attached to operations. The protected resource atomically compares it with its durable highest accepted token and rejects older operations, making superseded clients unable to overwrite newer effects.
Example. A has token 33 and pauses. B acquires 34 and writes. When A sends token 33 later, the resource rejects it because its fence is already 34.
Boundary. A new grant must be installed at the resource before it promises exclusion there. Token 33 can still pass until 34 is observed. Equal-token operations also need sequence/idempotency rules if they can arrive out of order.
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.
The distributed-correctness checklist: idempotency, the delivery model, ordering assumptions, races and locking, memory visibility, partial failure, deadline/retry propagation, and consistency — assume every step can happen twice or fail halfway.
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.
Wall clocks can skew/jump and do not by themselves establish causal order. Use logical versions/sequence or a reviewed bounded-clock protocol for correctness; use local monotonic deadlines for elapsed time where appropriate.
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:
# Pseudocode: a claim and its local contribution share ONE transaction.
begin transaction
claim = insert_event_if_absent(scope, event_id, payload_fingerprint)
if claim is new:
increment_aggregate(ad_id)
else:
validate_same_payload()
save_matching_partition_position()
commit
# A separate mark_seen followed by atomic_incr can lose the effect on crash.
Common follow-ups
How do you actually verify concurrency correctness in review?
How to answer
Ask for the invariant. Make the author state what must hold under concurrent access, then check the code enforces it.
A single-threaded test can prove/check a local modeled property but does not exercise interleavings, memory visibility, or distributed failures. Add deterministic schedules, stress/fault tests, and invariant reasoning.
Prefer designs that remove the race. Immutability, single-writer, or partition-by-key beats clever locking every time.
When is eventual consistency acceptable?
How to answer
Match it to the use case. Analytics counters and feeds tolerate eventual; money and access control usually don't.
Make the window explicit. The diff should state the convergence expectation and how readers cope with staleness.
Check the read path. Read-your-writes, monotonic reads — does the client need a guarantee the store doesn't give?
How do you review a change to a locking scheme?
How to answer
Lock ordering. All paths must acquire locks in the same order, or you have a latent deadlock.
Avoid long/blocking work under a lock where practical; if the protocol requires it, state deadlines, lock ordering, and the throughput/deadlock implications. This is a design constraint, not a universal syntactic ban.
Contention. A single coarse lock on the hot path is a throughput ceiling; consider sharding or lock-free structures.
What's your stance on wall-clock time in distributed code?
How to answer
Never for ordering or correctness. Clocks drift; NTP jumps. Use logical clocks, versions, or sequence numbers.
Fine for observability and coarse TTLs. Timestamps are okay for logs and rough expiry, not for “last write wins” across nodes without bounded skew reasoning.
Watch timeouts under skew. Deadlines computed on one node and checked on another can misbehave.
Evidence to collect with your approved tools
A decision/review record with the problem, constraints, options, and your contribution.
Relevant tests, dashboards, or measurements with definitions, denominators, dates, and uncertainty.
Incident or delivery records showing actual outcomes and unresolved limitations.
Stakeholder agreements and follow-up actions with owners and review dates.
Only authorized, minimized, safe-to-share evidence. Do not copy confidential personnel notes into a public story.
Worked example: the seen-marker crash gap
This is an illustrative situation. The point is the reasoning and evidence, not copying the numbers or first-person story.
Sequence for this example. The table explains the evidence and decision at each step.
Step
Evidence, decision, and expected result
Receive event e1
The desired effect is one increment for this logical event.
Mark e1 seen separately
If the worker crashes before incrementing, the retry is skipped and the event's effect is lost.
Commit claim and increment together
One transaction protects event identity, payload, contribution, and matching recovered position/state.
Replay safely
The same logical identity is a no-op after commit; a failed transaction leaves neither claim nor effect. Remote calls need their own idempotency/reconciliation protocol.
Check your understanding: Does atomic_incr after a separate mark_seen close the crash gap?
No. Two individually atomic operations are not one atomic transaction.
Run and change the example
Download guide_labs.py. Python 3.10+, standard library only. In its directory, run:
python3 guide_labs.py transactions
python3 guide_labs.py test
The SQLite model runs concurrent retries and rollback injections. It deliberately has no external side effects or expiring dedup keys.
The program tests its stated finite contracts. It is an educational model, not an implementation of the named production product or a proof of unmodeled distributed faults.
