Use advisory lock to prevent concurrent processing of the same review request#5718
Use advisory lock to prevent concurrent processing of the same review request#5718suhaibmujahid wants to merge 2 commits intomozilla:masterfrom
Conversation
There was a problem hiding this comment.
Pull request overview
This PR addresses issue #5716 by implementing recovery backoff and duplicate-avoidance mechanisms to prevent the Review Helper from processing the same diff multiple times and posting duplicate comments. The changes add timeout handling for Cloud Tasks, recovery logic for stuck processing states, and detection of concurrent processing attempts.
Changes:
- Added a 30-minute dispatch deadline for Cloud Tasks to prevent indefinite task execution
- Implemented recovery backoff logic that delays retries when a review request is stuck in PROCESSING state
- Added duplicate-avoidance by detecting when the client disconnects and another task has taken over processing
- Improved error handling to properly reset status on transient errors
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| services/reviewhelper-api/app/tasks.py | Added dispatch_deadline of 30 minutes to Cloud Tasks configuration |
| services/reviewhelper-api/app/routers/internal.py | Implemented recovery backoff logic, duplicate-avoidance via disconnection detection, and improved error handling |
| services/reviewhelper-api/app/review_processor.py | Modified to return tuple of (comments, summary, details) instead of modifying review_request directly |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
suhaibmujahid
force-pushed
the
duplicate-avoidance
branch
from
338c268 to
8885147
Compare
suhaibmujahid
changed the title
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
… request Remove task_id from review requests now that advisory lock prevents duplicates. The advisory lock mechanism makes the task_id-based deduplication unnecessary.
suhaibmujahid
force-pushed
the
duplicate-avoidance
branch
from
8885147 to
d39de9c
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
1 participant
Fixes #5716
Remove task_id from review requests now that advisory lock prevents duplicates. The advisory lock mechanism makes the task_id-based deduplication unnecessary.