Repository: scrapy/scrapy
Pull request #6828 (2025-05-27 06:44:43)

Scrapy 2.13.1: Prioritize other requests over start requests


Patch:
diff --git a/docs/news.rst b/docs/news.rst
index cf1c35893..eb5370b6e 100644
--- a/docs/news.rst
+++ b/docs/news.rst
@@ -3,6 +3,22 @@
 Release notes
 =============
 
+.. _release-2.13.1:
+
+Scrapy 2.13.1 (unreleased)
+--------------------------
+
+-   Give callback requests precedence over start requests when priority values
+    are the same.
+
+    This makes changes from 2.13.0 to start request handling more intuitive and
+    backward compatible. For scenarios where all requests have the same
+    priorities, in 2.13.0 all start requests were sent before the first
+    callback request. In 2.13.1, same as in 2.12 and lower, start requests are
+    only sent when there are not enough pending callback requests to reach
+    concurrency limits.
+
+
 .. _release-2.13.0:
 
 Scrapy 2.13.0 (2025-05-08)
diff --git a/scrapy/core/scheduler.py b/scrapy/core/scheduler.py
index 57d27b7cf..9ac447289 100644
--- a/scrapy/core/scheduler.py
+++ b/scrapy/core/scheduler.py
@@ -173,8 +173,8 @@ class Scheduler(BaseScheduler):
 
     :ref:`Start requests <start-requests>` are sent in the order they are
     yielded from :meth:`~scrapy.Spider.start`, and given the same
-    :attr:`~scrapy.http.Request.priority`, start requests take precedence over
-    other requests.
+    :attr:`~scrapy.http.Request.priority`, other requests take precedence over
+    start requests.
 
     You can set :setting:`SCHEDULER_START_MEMORY_QUEUE` and
     :setting:`SCHEDULER_START_DISK_QUEUE` to ``None`` to handle start requests
diff --git a/scrapy/pqueues.py b/scrapy/pqueues.py
index e6c6b8bf1..34b235d83 100644
--- a/scrapy/pqueues.py
+++ b/scrapy/pqueues.py
@@ -160,28 +160,32 @@ class ScrapyPriorityQueue:
 
     def pop(self) -> Request | None:
         while self.curprio is not None:
+            try:
+                q = self.queues[self.curprio]
+            except KeyError:
+                pass
+            else:
+                m = q.pop()
+                if not q:
+                    del self.queues[self.curprio]
+                    q.close()
+                    if not self._start_queues:
+                        self._update_curprio()
+                return m
             if self._start_queues:
                 try:
                     q = self._start_queues[self.curprio]
                 except KeyError:
-                    pass
+                    self._update_curprio()
                 else:
                     m = q.pop()
                     if not q:
                         del self._start_queues[self.curprio]
                         q.close()
+                        self._update_curprio()
                     return m
-            try:
-                q = self.queues[self.curprio]
-            except KeyError:
-                self._update_curprio()
             else:
-                m = q.pop()
-                if not q:
-                    del self.queues[self.curprio]
-                    q.close()
-                    self._update_curprio()
-                return m
+                self._update_curprio()
         return None
 
     def _update_curprio(self) -> None:


Test patch:
diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py
index 90af10f0e..c7dbc82d4 100644
--- a/tests/test_engine_loop.py
+++ b/tests/test_engine_loop.py
@@ -189,9 +189,9 @@ class RequestSendOrderTestCase(TestCase):
 
     @deferred_f_from_coro_f
     async def test_default(self):
-        """By default, start requests take priority over callback requests and
+        """By default, callback requests take priority over start requests and
         are sent in order. Priority matters, but given the same priority, a
-        start request takes precedence."""
+        callback request takes precedence."""
         nums = [1, 2, 3, 4, 5, 6]
         response_seconds = 0
         download_slots = 1
@@ -207,13 +207,13 @@ class RequestSendOrderTestCase(TestCase):
             yield _request(1)
 
             for request in (
-                _request(4, priority=1),
-                _request(6),
+                _request(2, priority=1),
+                _request(5),
             ):
                 spider.crawler.engine._slot.scheduler.enqueue_request(request)
-            yield _request(5)
-            yield _request(2, priority=1)
+            yield _request(6)
             yield _request(3, priority=1)
+            yield _request(4, priority=1)
 
         def parse(spider, response):
             return
@@ -249,13 +249,13 @@ class RequestSendOrderTestCase(TestCase):
             yield _request(1)
 
             for request in (
-                _request(4, priority=1),
-                _request(6),
+                _request(2, priority=1),
+                _request(5),
             ):
                 spider.crawler.engine._slot.scheduler.enqueue_request(request)
-            yield _request(5)
+            yield _request(6)
+            yield _request(4, priority=1)
             yield _request(3, priority=1)
-            yield _request(2, priority=1)
 
         def parse(spider, response):
             return
diff --git a/tests/test_pqueues.py b/tests/test_pqueues.py
index d5c710ed2..b65f1b7e7 100644
--- a/tests/test_pqueues.py
+++ b/tests/test_pqueues.py
@@ -7,6 +7,7 @@ from scrapy.http.request import Request
 from scrapy.pqueues import DownloaderAwarePriorityQueue, ScrapyPriorityQueue
 from scrapy.spiders import Spider
 from scrapy.squeues import FifoMemoryQueue
+from scrapy.utils.misc import build_from_crawler, load_object
 from scrapy.utils.test import get_crawler
 from tests.test_scheduler import MockDownloader, MockEngine
 
@@ -155,3 +156,56 @@ class TestDownloaderAwarePriorityQueue:
         assert self.queue.peek().url == req3.url
         assert self.queue.pop().url == req3.url
         assert self.queue.peek() is None
+
+
+@pytest.mark.parametrize(
+    ("input", "output"),
+    [
+        # By default, start requests are FIFO, other requests are LIFO.
+        ([{}, {}], [2, 1]),
+        ([{"start": True}, {"start": True}], [1, 2]),
+        # Priority matters.
+        ([{"priority": 1}, {"start": True}], [1, 2]),
+        ([{}, {"start": True, "priority": 1}], [2, 1]),
+        # For the same priority, start requests pop last.
+        ([{}, {"start": True}], [1, 2]),
+        ([{"start": True}, {}], [2, 1]),
+    ],
+)
+def test_pop_order(input, output):
+    def make_url(index):
+        return f"https://toscrape.com/{index}"
+
+    def make_request(index, data):
+        meta = {}
+        if data.get("start", False):
+            meta["is_start_request"] = True
+        return Request(
+            url=make_url(index),
+            priority=data.get("priority", 0),
+            meta=meta,
+        )
+
+    input_requests = [
+        make_request(index, data) for index, data in enumerate(input, start=1)
+    ]
+    expected_output_urls = [make_url(index) for index in output]
+
+    crawler = get_crawler(Spider)
+    settings = crawler.settings
+    queue = build_from_crawler(
+        ScrapyPriorityQueue,
+        crawler,
+        downstream_queue_cls=load_object(settings["SCHEDULER_MEMORY_QUEUE"]),
+        key="",
+        start_queue_cls=load_object(settings["SCHEDULER_START_MEMORY_QUEUE"]),
+    )
+
+    for request in input_requests:
+        queue.push(request)
+
+    actual_output_urls = []
+    while request := queue.pop():
+        actual_output_urls.append(request.url)
+
+    assert actual_output_urls == expected_output_urls