From 19c910f731cd9471b09f95e5548e1206626c363c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Adri=C3=A1n=20Chaves?= Date: Tue, 25 Mar 2025 15:28:21 +0100 Subject: [PATCH] Complete the new loop implementation (no lazy support yet) --- docs/news.rst | 19 ++++++-- docs/topics/spiders.rst | 67 +++++++++------------------- scrapy/core/engine.py | 5 ++- scrapy/spiders/__init__.py | 2 +- tests/test_engine_loop.py | 90 ++++++++++++++++++++++++++++++-------- 5 files changed, 113 insertions(+), 70 deletions(-) diff --git a/docs/news.rst b/docs/news.rst index 37ed1e693..fd77fb6b2 100644 --- a/docs/news.rst +++ b/docs/news.rst @@ -11,13 +11,18 @@ Scrapy VERSION (unreleased) Highlights: - Replaced ``start_requests()`` (sync) with :meth:`~scrapy.Spider.start` - (async) and changed how it is iterated by default. + (async) and changed how it is iterated. Backward-incompatible changes ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ -- By default, the iteration of start requests and items no longer stops once - there are requests in the scheduler. +- The iteration of start requests and items no longer stops once there are + requests in the scheduler, and instead runs continuously until all start + requests have been scheduled. + + As a result, the order in which start requests are sent may change. See + :ref:`start-requests` for details and information on how to force start + request order. - In ``scrapy.core.engine.ExecutionEngine``: @@ -66,6 +71,14 @@ New features (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) +- Start requests are now :ref:`scheduled ` as soon as + possible. + + As a result, their :attr:`~scrapy.Request.priority` is now taken into + account as soon as :setting:`CONCURRENT_REQUESTS` is reached. + + (:issue:`456`, :issue:`3477`, :issue:`4467`, :issue:`5627`, :issue:`6729`) + Bug fixes ~~~~~~~~~ diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index bc1c58ead..3d6216d60 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -364,63 +364,38 @@ used by :class:`~scrapy.downloadermiddlewares.useragent.UserAgentMiddleware`:: Spider arguments can also be passed through the Scrapyd ``schedule.json`` API. See `Scrapyd documentation`_. -.. _spider-start: +.. _start-requests: -Spider start -============ +Start requests +============== -The way :meth:`~scrapy.Spider.start` works by default may be counterintuitive: +Scrapy does not try to send :meth:`~scrapy.Spider.start` requests in order. +Instead, it prioritizes reaching :setting:`CONCURRENT_REQUESTS` and +:ref:`scheduling ` start requests. -#. First, the first 16 start requests are sent in order. +To change that, override the :meth:`~scrapy.Spider.start` method to set +:attr:`Request.priority `. For example: - That number depends on :setting:`CONCURRENT_REQUESTS`. :ref:`Awaiting - ` slow operations in :meth:`~scrapy.Spider.start` may lower it. - -#. Then, the last 8 start requests are sent in reverse order. - - That number depends on both :setting:`CONCURRENT_REQUESTS` and - :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`. Specifically, assuming an even - domain distribution in start requests (i.e. ABCABC, not AABBCC), it is: +- To send start requests before other requests: .. code-block:: python - min(CONCURRENT_REQUESTS, CONCURRENT_REQUESTS_PER_DOMAIN * domain_count) + async def start(self): + async for request in super().start(): + yield request.replace(priority=1) -#. Finally, the remaining start requests are also sent in reverse order, but - only when there are not enough pending requests yielded from callbacks to - reach the configured concurrency. +- To send start requests in order: -.. note:: Response order is a different story: it is determined not only by - request order, but also by response time. + .. code-block:: python -The reverse order is because the :ref:`scheduler `, which -handles pending requests, stores requests with the same -:attr:`~scrapy.Request.priority` in a LIFO (last in, first out) queue by -default (see :setting:`SCHEDULER_MEMORY_QUEUE` and -:setting:`SCHEDULER_DISK_QUEUE`). The order of the first few requests is -unnaffected because they are sent as soon as they are scheduled, and the last -start requests sent before callback requests are those that can be sent before -the first callback requests are scheduled. + async def start(self): + priority = len(self.start_urls) + async for request in super().start(): + yield request.replace(priority=priority) + priority -= 1 -If you need start requests to be sent before requests yielded from spider -callbacks, you can set a higher priority for them. For example: - -.. code-block:: python - - async def start(self): - async for request in super().start(): - yield request.replace(priority=1) - -If you also need them to be sent in order, you can assign them decreasing -priority values. For example: - -.. code-block:: python - - async def start(self): - priority = len(self.start_urls) - async for request in super().start(): - yield request.replace(priority=priority) - priority -= 1 +You can also :ref:`customize the scheduler ` if you need +more control over request prioritization. .. _builtin-spiders: diff --git a/scrapy/core/engine.py b/scrapy/core/engine.py index 6ec9780e6..63b332563 100644 --- a/scrapy/core/engine.py +++ b/scrapy/core/engine.py @@ -45,13 +45,13 @@ class _Slot: def __init__( self, close_if_idle: bool, - nextcall: CallLaterOnce[Deferred[None]], + nextcall: CallLaterOnce[None], scheduler: BaseScheduler, ) -> None: self.closing: Deferred[None] | None = None self.inprogress: set[Request] = set() self.close_if_idle: bool = close_if_idle - self.nextcall: CallLaterOnce[Deferred[None]] = nextcall + self.nextcall: CallLaterOnce[None] = nextcall self.scheduler: BaseScheduler = scheduler self.heartbeat: LoopingCall = LoopingCall(nextcall.schedule) @@ -191,6 +191,7 @@ class ExecutionEngine: Items are scraped. Requests are scheduled. """ + assert self._slot is not None # typing while self._start is not None: yield self._process_next_spider_start_yield() if not self._needs_backout(): diff --git a/scrapy/spiders/__init__.py b/scrapy/spiders/__init__.py index 30e197b8c..80932bc9d 100644 --- a/scrapy/spiders/__init__.py +++ b/scrapy/spiders/__init__.py @@ -122,7 +122,7 @@ class Spider(object_ref): def start_requests(self): yield Request("https://toscrape.com/") - .. seealso:: :ref:`spider-start` + .. seealso:: :ref:`start-requests` """ for item_or_request in self.start_requests(): yield item_or_request diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 058deee8b..0dc053208 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -81,7 +81,34 @@ class MainTestCase(TestCase): assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" -class MockServerTestCase(TestCase): +class RequestSendOrderTestCase(TestCase): + """Test the intrincacies of request send order when all start requests and + callback requests have the same priority. + + It is a very unintuitive behavior, documented as “undefined” so that we may + change it in the future without breaking the contract. + + 1. First, the first CONCURRENT_REQUESTS start requests are sent in order. + + Awaiting slow operations in Spider.start() can lower that. + + 2. Then, assuming an even domain distribution in start requests (i.e. + ABCABC, not AABBCC), the last N start requests are sent in reverse + order, where N is: + + min(CONCURRENT_REQUESTS, CONCURRENT_REQUESTS_PER_DOMAIN * domain_count) + + 3. Finally, the remaining start requests are also sent in reverse order, + but only when there are not enough pending requests yielded from + callbacks to reach the configured concurrency. + + The reverse order is because the scheduler uses a LIFO queue by default + (SCHEDULER_MEMORY_QUEUE, SCHEDULER_DISK_QUEUE). The order of the first few + requests is unnaffected because they are sent as soon as they are + scheduled, and the last start requests sent before callback requests are + those that can be sent before the first callback requests are scheduled. + """ + @classmethod def setUpClass(cls): cls.mockserver = MockServer() @@ -91,12 +118,14 @@ class MockServerTestCase(TestCase): def tearDownClass(cls): cls.mockserver.__exit__(None, None, None) - # Verify the default behavior of the engine loop as described in the docs, - # in the “Spider start” section of the page about spdiers. - fast_seconds = 0.001 slow_seconds = 0.2 # increase if flaky + def _request(self, num, response_seconds, download_slots): + url = self.mockserver.url(f"/delay?n={response_seconds}&{num}") + meta = {"download_slot": str(num % download_slots)} + return Request(url, meta=meta) + @deferred_f_from_coro_f async def _test_request_order( self, @@ -105,22 +134,26 @@ class MockServerTestCase(TestCase): settings=None, response_seconds=None, download_slots=1, + start_fn=None, ): settings = settings or {} response_seconds = response_seconds or self.slow_seconds - def _request(num): - url = self.mockserver.url(f"/delay?n={response_seconds}&{num}") - meta = {"download_slot": str(num % download_slots)} - return Request(url, meta=meta) + if start_fn is None: + + async def start_fn(spider): + for num in start_nums: + yield self._request(num, response_seconds, download_slots) class TestSpider(Spider): name = "test" - cb_requests = deque([_request(num) for num in cb_nums]) - - async def start(self): - for num in start_nums: - yield _request(num) + cb_requests = deque( + [ + self._request(num, response_seconds, download_slots) + for num in cb_nums + ] + ) + start = start_fn def parse(self, response): while self.cb_requests: @@ -344,6 +377,8 @@ class MockServerTestCase(TestCase): @deferred_f_from_coro_f async def test_fast(self): + """Very fast responses may increase the number of start requests sent + in reverse order before the first callback request.""" await maybe_deferred_to_future( self._test_request_order( start_nums=[1, 3, 2], @@ -352,9 +387,28 @@ class MockServerTestCase(TestCase): response_seconds=self.fast_seconds, ) ) - # TODO: Test how increasing concurrency behaves with fast responses. - # TODO: Test claims: - # - :ref:`Awaiting ` slow operations in :meth:`~scrapy.Spider.start` may lower it. - # - If responses are very fast, it can be more than :setting:`CONCURRENT_REQUESTS`. - # - Otherwise, it can reach 16 (:setting:`CONCURRENT_REQUESTS`) + @deferred_f_from_coro_f + async def test_await(self): + """Awaiting slow operations in Spider.start() may lower the number of + first start requests sent in order.""" + start_nums = [1, 3] + response_seconds = self.slow_seconds + download_slots = 1 + + async def start(spider): + assert len(start_nums) > 1 + for num in start_nums[:-1]: + yield self._request(num, response_seconds, download_slots) + await sleep(response_seconds * 2) + yield self._request(start_nums[-1], response_seconds, download_slots) + + await maybe_deferred_to_future( + self._test_request_order( + start_nums=start_nums, + cb_nums=[2], + settings={"CONCURRENT_REQUESTS": 2}, + response_seconds=response_seconds, + start_fn=start, + ) + )