Skip to content

fix: prolong and track shared request queue locks to prevent duplicate processing - #1062

Merged
vdusek merged 6 commits into
masterfrom
worktree-fix-b2
Aug 19, 2026
Merged

fix: prolong and track shared request queue locks to prevent duplicate processing#1062
vdusek merged 6 commits into
masterfrom
worktree-fix-b2

Conversation

@vdusek

@vdusek vdusek commented Jul 21, 2026

Copy link
Copy Markdown
Contributor

The shared request-queue client (access='shared') locked a batch of up to 25 requests for 180 s in list_and_lock_head, but never tracked, prolonged, or released those locks. As a batch drained slowly, locks expired mid-flight and requests got processed twice - by another consumer that re-locked an expired request, or by a second worker in this process after the request was re-listed. That defeats the class's documented "request locking to prevent duplicate processing" guarantee (double charges in PPE, duplicate dataset items).

Changes:

  • Store lock_expires_at per cached head entry (the API returned it, we discarded it).
  • Track handed-out requests in _requests_in_progress: skip them when re-listing the head and when fetching, and keep is_finished false while any is outstanding.
  • In fetch_next_request, skip candidates whose lock already lapsed, and prolong the lock only when its remaining window is below half the lock time (_MIN_LOCK_TIME_AT_HANDOFF, 90 s). A request handed out from a fresh batch lock needs no prolong, so the common path adds no API write and shared mode stays at 3 writes per request.

Out of scope: handlers outliving the lock window would need a background refresh loop, and deleting leftover head locks on shutdown has no RequestQueueClient hook to attach to (they lapse within the lock TTL anyway).

✍️ Drafted by Claude Code

@vdusek vdusek added adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. labels Jul 21, 2026
@vdusek vdusek self-assigned this Jul 21, 2026
@github-actions github-actions Bot added this to the 145th sprint - Tooling team milestone Jul 21, 2026
@github-actions github-actions Bot added the tested Temporary label used only programatically for some analytics. label Jul 21, 2026
@codecov

codecov Bot commented Jul 21, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 91.30435% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 91.87%. Comparing base (58d6da1) to head (a47a37d).
⚠️ Report is 34 commits behind head on master.

Files with missing lines Patch % Lines
...age_clients/_apify/_request_queue_shared_client.py 91.30% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1062      +/-   ##
==========================================
- Coverage   91.89%   91.87%   -0.02%     
==========================================
  Files          51       51              
  Lines        3232     3274      +42     
==========================================
+ Hits         2970     3008      +38     
- Misses        262      266       +4     
Flag Coverage Δ
e2e 34.97% <6.52%> (-0.37%) ⬇️
integration 57.02% <56.52%> (-0.07%) ⬇️
unit 85.46% <91.30%> (+2.20%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vdusek

vdusek commented Jul 21, 2026

Copy link
Copy Markdown
Contributor Author

Btw. this is a different case than #887

@vdusek
vdusek marked this pull request as ready for review July 21, 2026 18:47
@vdusek
vdusek requested a review from Pijukatel July 21, 2026 18:47
Comment thread src/apify/storage_clients/_apify/_request_queue_shared_client.py Outdated
@vdusek
vdusek requested a review from Pijukatel July 30, 2026 09:05
@vdusek
vdusek merged commit ccaad7b into master Aug 19, 2026
31 checks passed
@vdusek
vdusek deleted the worktree-fix-b2 branch August 19, 2026 07:37
vdusek added a commit that referenced this pull request Aug 19, 2026
Combines the lock tracking from #1062 with the `is_finished` verification here:

- `is_finished` keeps the cheap early-out (empty head, no platform locks, no
  locally in-progress request) and only then confirms the empty-head verdict
  against per-request reads.
- `reclaim_request` reuses the `request_id` that #1062 now binds before the
  try block for `_requests_in_progress`.
- Tests: kept both sets; #1062's always-unhandled `_client_request` helper was
  dropped in favour of the existing one that takes `handled_at`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

adhoc Ad-hoc unplanned task added during the sprint. t-tooling Issues with this label are in the ownership of the tooling team. tested Temporary label used only programatically for some analytics.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants