Code review exercise: find concurrency, failure and ownership bugs in a small job queue

This round hands you a small coding project and asks you to review it as you would a colleague's work: find the gaps, the implementation problems and the API issues, explain why each one matters, and say what you would change. The project used in the interview is not reproduced here. For practice, review the job queue module below, in which producers submit jobs and several consumer threads claim and process them. Assume it is meant to run inside one long-running service process. ```python import threading import time import uuid class Job: def __init__(self, payload, max_retries=3): self.id = str(uuid.uuid4()) self.payload = payload self.max_retries = max_retries self.attempts = 0 self.status = "queued" # queued | running | done | failed self.owner = None class JobQueue: def __init__(self): self.jobs = [] self.lock = threading.Lock() def submit(self, payload, max_retries=3): job = Job(payload, max_retries) self.jobs.append(job) return job def claim(self, consumer_id): for job in self.jobs: if job.status == "queued": with self.lock: job.status = "running" job.owner = consumer_id return job return None def complete(self, job_id, consumer_id): job = self._find(job_id) job.status = "done" def fail(self, job_id, consumer_id): job = self._find(job_id) job.attempts += 1 if job.attempts < job.max_retries: job.status = "queued" job.status = "failed" def get_jobs(self): return self.jobs def _find(self, job_id): for job in self.jobs: if job.id == job_id: return job def consumer_loop(queue, consumer_id, handler): while True: job = queue.claim(consumer_id) if job is None: time.sleep(1) continue try: handler(job.payload) except Exception: pass queue.complete(job.id, consumer_id) ``` ### Constraints and Clarifications - Treat this as a real review: findings range from merge-blocking defects to minor polish, and you should say which is which. - Intended behavior: each submitted job is processed by one consumer at a time, a failed job is retried a limited number of times and then marked failed, and callers can check a job's status. - The module does not claim durability across process restarts; decide whether to raise it. ### Clarifying Questions - Is processing a job twice harmful, or are the handlers idempotent? - What should happen to a job whose consumer dies halfway through? - Who calls `get_jobs`, and what do they need from it? - Roughly how many jobs and consumers are expected? ### Part 1 — Find the problems List the problems you see, classify each as blocking or non-blocking, and explain the consequence of each one. ```hint Read it as two consumers Trace two consumer threads and one failing handler through the code at the same time. ``` #### What This Part Should Cover - Concurrency defects - Failure-handling defects, including the retry logic - API and ownership problems - A clear severity classification ### Part 2 — Fix the API and ownership model Propose a corrected interface and show corrected code for claiming a job and for reporting success or failure. ```hint Prove you still own it Think about what a consumer should present when it reports a result, so that a late or stale report can be rejected. ``` #### What This Part Should Cover - An atomic claim - A proof of ownership on completion and failure - Recovery of work from consumers that die - Explicit errors and correct retry semantics ### Part 3 — Tests and verdict Which tests would have caught these problems, and what is your verdict on the change as written? ```hint Test the unhappy paths List the scenarios the original code gets wrong and turn each into a deterministic test. ``` #### What This Part Should Cover - Deterministic tests for concurrency and failure paths - Time-dependent behavior tested without real sleeps - A clear verdict with the required changes ### What a Strong Answer Covers - Correctness and failure handling prioritized over style - Each finding tied to a concrete consequence (double processing, lost failures, stranded jobs) - A corrected design that fixes the root causes rather than patching symptoms - Tests that would fail on the original code - A clear verdict and priority order the author can act on ### Follow-up Questions - The handlers cannot be made idempotent. What guarantees can the queue offer, and at what cost? - How would you make the queue survive a process restart? - How would you add per-job priorities without starving low-priority work? - How would you review this differently if the author were new to concurrent code?