Skip to content

fix: eagerly update local head on forefront ops in shared RequestQueue#887

Closed
vdusek wants to merge 1 commit into
masterfrom
fix/forefront-reclaim-shared-rq
Closed

fix: eagerly update local head on forefront ops in shared RequestQueue#887
vdusek wants to merge 1 commit into
masterfrom
fix/forefront-reclaim-shared-rq

Conversation

@vdusek
Copy link
Copy Markdown
Contributor

@vdusek vdusek commented May 4, 2026

Summary

The shared RequestQueue client relied on list_and_lock_head to reflect forefront updates from update_request(forefront=true) and batch_add_requests(forefront=true). Because the Apify API has a propagation delay between those operations and list_and_lock_head (#808), the next fetch could return a stale request — the failing CI for test_request_reclaim_with_forefront[shared] returned https://example.com/2 instead of the just-reclaimed /1.

This mirrors the single-mode client (_request_queue_single_client.py:262) by eagerly updating the local _queue_head deque on forefront operations:

  • reclaim_request(forefront=True)appendleft the reclaimed request_id (with contextlib.suppress(ValueError) dedupe). Removed the now-redundant _should_check_for_forefront_requests = True flag setting.
  • add_batch_of_requests(forefront=True)appendleft newly-added (non-unprocessed) request IDs in reverse order so the first batched item ends up at the front. Also passed hydrated_request=request to _cache_request so subsequent fetches don't need an extra API roundtrip.
  • Removed _should_check_for_forefront_requests entirely (and its leftover-buffer logic in _list_head). With eager local updates the cache is authoritative for our own forefront ops; natural drain (len(_queue_head) ≤ 1) handles the refresh for picking up other clients' changes.

The motivation for the change is to fix this flaky test in CI: https://github.com/apify/apify-sdk-python/actions/runs/25316848848/job/74216228877

The shared `RequestQueue` client relied on `list_and_lock_head` to reflect
forefront updates from `update_request(forefront=true)` and
`batch_add_requests(forefront=true)`, but the API has propagation delay
between those operations. The next fetch could return a different request,
causing flaky test failures (#808). Mirror the single-mode client by
eagerly inserting forefront request IDs at the front of the local
`_queue_head` cache instead of relying on an API roundtrip.
@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 May 4, 2026
@vdusek vdusek self-assigned this May 4, 2026
@github-actions github-actions Bot added this to the 140th sprint - Tooling team milestone May 4, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented May 4, 2026

Codecov Report

❌ Patch coverage is 7.14286% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 75.74%. Comparing base (6f82da9) to head (73a6568).
⚠️ Report is 2 commits behind head on master.

Files with missing lines Patch % Lines
...age_clients/_apify/_request_queue_shared_client.py 7.14% 13 Missing ⚠️

❗ There is a different number of reports uploaded between BASE (6f82da9) and HEAD (73a6568). Click for more details.

HEAD has 1 upload less than BASE
Flag BASE (6f82da9) HEAD (73a6568)
integration 1 0
Additional details and impacted files
@@             Coverage Diff             @@
##           master     #887       +/-   ##
===========================================
- Coverage   86.94%   75.74%   -11.21%     
===========================================
  Files          48       48               
  Lines        2942     2944        +2     
===========================================
- Hits         2558     2230      -328     
- Misses        384      714      +330     
Flag Coverage Δ
e2e 37.73% <7.14%> (+<0.01%) ⬆️
integration ?
unit 75.74% <7.14%> (+0.05%) ⬆️

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

☔ View full report in Codecov by Sentry.
📢 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 closed this May 15, 2026
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants