diff --git a/docs/topics/spiders.rst b/docs/topics/spiders.rst index 2432223eb..25ce81b2e 100644 --- a/docs/topics/spiders.rst +++ b/docs/topics/spiders.rst @@ -369,31 +369,35 @@ See `Scrapyd documentation`_. Spider start ============ -The way :meth:`~scrapy.Spider.start` works may be counterintuitive. +The way :meth:`~scrapy.Spider.start` works by default may be counterintuitive: -By default, if you do not use :ref:`await ` in -:meth:`~scrapy.Spider.start`, or if you instead use the -:attr:`~scrapy.Spider.start_urls` attribute, this is what happens: +#. The first 8-16 start requests are sent in the order in which they are + yielded. -- The first 8-16 start requests (based on :setting:`CONCURRENT_REQUESTS` and - :setting:`CONCURRENT_REQUESTS_PER_DOMAIN`) are sent in the order in which - they are yielded. + That number depends on :setting:`CONCURRENT_REQUESTS`. + :ref:`Awaiting ` slow operations in :meth:`~scrapy.Spider.start` may + lower it. - .. note:: Responses may come in a different order. +#. The last start request is sent. -- The remaining start requests are sent in reverse order, and only when there - are not enough pending requests yielded from callbacks to reach the + It could be more start requests for very low response times. If so, they + are the last start requests in reverse order. + +#. 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. - This is because the :ref:`scheduler `, where pending - requests are stored, uses a LIFO (last in, first out) queue by default, - configured in the :setting:`SCHEDULER_MEMORY_QUEUE` and - :setting:`SCHEDULER_DISK_QUEUE`. +.. note:: Response order is a different story: it is determined not only by + request order, but also by response time. - So, provided all pending requests have the same - :attr:`~scrapy.Request.priority`, scheduled requests are sent in reserve - order. The first few requests are sent in order only because they are sent - as soon as they are scheduled. +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. If you need start requests to be sent before requests yielded from spider callbacks, you can set a higher priority for them. For example: diff --git a/tests/test_engine_loop.py b/tests/test_engine_loop.py index 62e941f49..fce20eda9 100644 --- a/tests/test_engine_loop.py +++ b/tests/test_engine_loop.py @@ -82,9 +82,6 @@ class MainTestCase(TestCase): class MockServerTestCase(TestCase): - # See the comment on the matching line above. - timeout = ExecutionEngine._SLOT_HEARTBEAT_INTERVAL - @classmethod def setUpClass(cls): cls.mockserver = MockServer() @@ -94,37 +91,185 @@ class MockServerTestCase(TestCase): def tearDownClass(cls): cls.mockserver.__exit__(None, None, None) - @deferred_f_from_coro_f - async def test_default_behavior(self): - """Verify the behavior that the docs claims is the default when it - comes to start request send order.""" - seconds = 0.1 + # Verify the default behavior of the engine loop as described in the docs, + # in the “Spider start” section of the page about spdiers. + # + # TODO: Check how combinations of the following parameters affect the + # behavior: response time (high/low), max concurrency (hihg/low), number of + # start requests (few/many), and number of domains (single/multiple). - def _url(id, delay=seconds): - return self.mockserver.url(f"/delay?n={seconds}&{id}") + fast_seconds = 0.001 + slow_seconds = 0.2 # increase if flaky + + @deferred_f_from_coro_f + async def _test_request_order( + self, + start_nums, + cb_nums, + settings=None, + response_seconds=None, + download_slots=1, + ): + 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) class TestSpider(Spider): name = "test" - start_urls = [_url("a", delay=0), _url("b"), _url("d"), _url("e")] - queue = deque([Request(_url("c"))]) + cb_requests = deque([_request(num) for num in cb_nums]) + + async def start(self): + for num in start_nums: + yield _request(num) def parse(self, response): - try: - request = self.queue.popleft() - except IndexError: - pass - else: - yield request + while self.cb_requests: + yield self.cb_requests.popleft() - actual_urls = [] + actual_nums = [] - def track_url(request, spider): - actual_urls.append(request.url) + def track_num(request, spider): + actual_nums.append(int(request.url.rsplit("&", maxsplit=1)[1])) - settings = {"CONCURRENT_REQUESTS": 2} crawler = get_crawler(TestSpider, settings_dict=settings) - crawler.signals.connect(track_url, signals.request_reached_downloader) + crawler.signals.connect(track_num, signals.request_reached_downloader) await maybe_deferred_to_future(crawler.crawl()) assert crawler.stats.get_value("finish_reason") == "finished" - expected_urls = [_url(letter) for letter in "abcd"] - assert actual_urls == expected_urls, f"{actual_urls=} != {expected_urls=}" + expected_nums = sorted(start_nums + cb_nums) + assert actual_nums == expected_nums, f"{actual_nums=} != {expected_nums=}" + + # TODO: Figure out why the behavior changes when CONCURRENT_REQUESTS is + # higher than CONCURRENT_REQUESTS_PER_DOMAIN. The number of requests sent + # before callback requests seems to depend on CONCURRENT_REQUESTS and not + # in CONCURRENT_REQUESTS_PER_DOMAIN. + @deferred_f_from_coro_f + async def test_default(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 19, + 17, + 16, + 15, + 14, + 13, + 12, + 11, + 10, + ], + cb_nums=[18], + settings={"CONCURRENT_REQUESTS": 9}, + ) + ) + + @deferred_f_from_coro_f + async def test_conc1(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 4, 2], + cb_nums=[3], + settings={"CONCURRENT_REQUESTS": 1}, + ) + ) + + @deferred_f_from_coro_f + async def test_conc2(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 2, 6, 4, 3], + cb_nums=[5], + settings={"CONCURRENT_REQUESTS": 2}, + ) + ) + + @deferred_f_from_coro_f + async def test_conc8(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 2, 3, 4, 5, 6, 7, 8, 18, 16, 15, 14, 13, 12, 11, 10, 9], + cb_nums=[17], + settings={"CONCURRENT_REQUESTS": 8}, + ) + ) + + @deferred_f_from_coro_f + async def test_conc16(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[ + 1, + 2, + 3, + 4, + 5, + 6, + 7, + 8, + 9, + 10, + 11, + 12, + 13, + 14, + 15, + 16, + 34, + 32, + 31, + 30, + 29, + 28, + 27, + 26, + 25, + 24, + 23, + 22, + 21, + 20, + 19, + 18, + 17, + ], + cb_nums=[33], + settings={"CONCURRENT_REQUESTS_PER_DOMAIN": 16}, + ) + ) + + @deferred_f_from_coro_f + async def test_domains(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 2, 6, 4, 3], + cb_nums=[5], + settings={ + "CONCURRENT_REQUESTS": 2, + "CONCURRENT_REQUESTS_PER_DOMAIN": 1, + }, + download_slots=2, + ) + ) + + @deferred_f_from_coro_f + async def test_fast(self): + await maybe_deferred_to_future( + self._test_request_order( + start_nums=[1, 3, 2], + cb_nums=[4], + settings={"CONCURRENT_REQUESTS": 1}, + response_seconds=self.fast_seconds, + ) + )