Merge 953c5d8a7b into e28e56aa61

docs/topics/security.rst

@@ -276,3 +276,31 @@ and a crawl that spans multiple domains can leak them to unintended hosts:
   default :setting:`REFERRER_POLICY` already avoids sending the referrer from
   HTTPS to HTTP, but you can tighten it further (for example, to
   ``same-origin`` or ``no-referrer``) if needed.
+
+.. _security-job-state:
+
+Job state integrity
+===================
+
+When you use :setting:`JOBDIR` to :ref:`pause and resume crawls
+<topics-jobs>`, the default scheduler disk queues
+(:setting:`SCHEDULER_DISK_QUEUE`, :setting:`SCHEDULER_START_DISK_QUEUE`) write
+requests to a compact binary format that is only guaranteed to be complete
+after a clean shutdown. If the process is killed, runs out of disk space, or
+shares its job directory with a second process, those files can be left
+truncated, and resuming the crawl then either fails with an unrecoverable
+error or silently drops the pending requests.
+
+To store the scheduler queues in an SQLite database instead, where each
+request is written within its own transaction:
+
+.. code-block:: python
+
+    SCHEDULER_DISK_QUEUE = "scrapy.squeues.PickleLifoSQLiteQueue"
+    SCHEDULER_START_DISK_QUEUE = "scrapy.squeues.PickleFifoSQLiteQueue"
+
+* **Pro:** an interrupted write cannot corrupt the queues or discard pending
+  requests unnoticed.
+
+* **Con:** scheduling requests becomes slower, which can be noticeable in
+  crawls with a high request throughput.

docs/topics/settings.rst

@@ -1940,10 +1940,21 @@ SCHEDULER_DISK_QUEUE
 
 Default: ``'scrapy.squeues.PickleLifoDiskQueue'``
 
+.. versionadded:: VERSION
+   The ``SQLite`` queue types.
+
 Type of disk queue that will be used by the scheduler. Other available types
 are ``scrapy.squeues.PickleFifoDiskQueue``,
 ``scrapy.squeues.MarshalFifoDiskQueue``,
-``scrapy.squeues.MarshalLifoDiskQueue``.
+``scrapy.squeues.MarshalLifoDiskQueue``,
+``scrapy.squeues.PickleFifoSQLiteQueue``,
+``scrapy.squeues.PickleLifoSQLiteQueue``,
+``scrapy.squeues.MarshalFifoSQLiteQueue`` and
+``scrapy.squeues.MarshalLifoSQLiteQueue``.
+
+The ``SQLite`` types store requests in an SQLite database, which makes writes
+slower but keeps the queue usable after an unclean shutdown. See
+:ref:`security-job-state`.
 
 
 .. setting:: SCHEDULER_MEMORY_QUEUE

scrapy/squeues.py

@@ -166,11 +166,39 @@ _MarshalLifoSerializationDiskQueue = _serializable_queue(
     marshal.dumps,
     marshal.loads,
 )
+_PickleFifoSerializationSQLiteQueue = _serializable_queue(
+    _with_mkdir(queue.FifoSQLiteQueue),  # type: ignore[arg-type]
+    _pickle_serialize,
+    pickle.loads,
+)
+_PickleLifoSerializationSQLiteQueue = _serializable_queue(
+    _with_mkdir(queue.LifoSQLiteQueue),  # type: ignore[arg-type]
+    _pickle_serialize,
+    pickle.loads,
+)
+_MarshalFifoSerializationSQLiteQueue = _serializable_queue(
+    _with_mkdir(queue.FifoSQLiteQueue),  # type: ignore[arg-type]
+    marshal.dumps,
+    marshal.loads,
+)
+_MarshalLifoSerializationSQLiteQueue = _serializable_queue(
+    _with_mkdir(queue.LifoSQLiteQueue),  # type: ignore[arg-type]
+    marshal.dumps,
+    marshal.loads,
+)
 
 # public queue classes
 PickleFifoDiskQueue = _scrapy_serialization_queue(_PickleFifoSerializationDiskQueue)
 PickleLifoDiskQueue = _scrapy_serialization_queue(_PickleLifoSerializationDiskQueue)
 MarshalFifoDiskQueue = _scrapy_serialization_queue(_MarshalFifoSerializationDiskQueue)
 MarshalLifoDiskQueue = _scrapy_serialization_queue(_MarshalLifoSerializationDiskQueue)
+PickleFifoSQLiteQueue = _scrapy_serialization_queue(_PickleFifoSerializationSQLiteQueue)
+PickleLifoSQLiteQueue = _scrapy_serialization_queue(_PickleLifoSerializationSQLiteQueue)
+MarshalFifoSQLiteQueue = _scrapy_serialization_queue(
+    _MarshalFifoSerializationSQLiteQueue
+)
+MarshalLifoSQLiteQueue = _scrapy_serialization_queue(
+    _MarshalLifoSerializationSQLiteQueue
+)
 FifoMemoryQueue = _scrapy_non_serialization_queue(queue.FifoMemoryQueue)  # type: ignore[arg-type]
 LifoMemoryQueue = _scrapy_non_serialization_queue(queue.LifoMemoryQueue)  # type: ignore[arg-type]

tests/test_squeues.py

@@ -10,9 +10,13 @@ from scrapy.loader import ItemLoader
 from scrapy.selector import Selector
 from scrapy.squeues import (
     _MarshalFifoSerializationDiskQueue,
+    _MarshalFifoSerializationSQLiteQueue,
     _MarshalLifoSerializationDiskQueue,
+    _MarshalLifoSerializationSQLiteQueue,
     _PickleFifoSerializationDiskQueue,
+    _PickleFifoSerializationSQLiteQueue,
     _PickleLifoSerializationDiskQueue,
+    _PickleLifoSerializationSQLiteQueue,
 )
 
 
@@ -45,7 +49,7 @@ def nonserializable_object_test(self):
     q.close()
 
 
-class FifoDiskQueueTestMixin:
+class FifoQueueTestMixin:
     def test_serialize(self):
         q = self.queue()
         q.push("a")
@@ -59,7 +63,7 @@ class FifoDiskQueueTestMixin:
     test_nonserializable_object = nonserializable_object_test
 
 
-class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin):
+class MarshalFifoDiskQueueTest(t.FifoDiskQueueTest, FifoQueueTestMixin):
     chunksize = 100000
 
     def queue(self):
@@ -82,7 +86,7 @@ class ChunkSize4MarshalFifoDiskQueueTest(MarshalFifoDiskQueueTest):
     chunksize = 4
 
 
-class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoDiskQueueTestMixin):
+class PickleFifoDiskQueueTest(t.FifoDiskQueueTest, FifoQueueTestMixin):
     chunksize = 100000
 
     def queue(self):
@@ -154,7 +158,7 @@ class ChunkSize4PickleFifoDiskQueueTest(PickleFifoDiskQueueTest):
     chunksize = 4
 
 
-class LifoDiskQueueTestMixin:
+class LifoQueueTestMixin:
     def test_serialize(self):
         q = self.queue()
         q.push("a")
@@ -168,12 +172,12 @@ class LifoDiskQueueTestMixin:
     test_nonserializable_object = nonserializable_object_test
 
 
-class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin):
+class MarshalLifoDiskQueueTest(t.LifoDiskQueueTest, LifoQueueTestMixin):
     def queue(self):
         return _MarshalLifoSerializationDiskQueue(self.qpath)
 
 
-class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin):
+class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoQueueTestMixin):
     def queue(self):
         return _PickleLifoSerializationDiskQueue(self.qpath)
 
@@ -206,3 +210,23 @@ class PickleLifoDiskQueueTest(t.LifoDiskQueueTest, LifoDiskQueueTestMixin):
         assert r.url == r2.url
         assert r2.meta["request"] is r2
         q.close()
+
+
+class MarshalFifoSQLiteQueueTest(t.FifoSQLiteQueueTest, FifoQueueTestMixin):
+    def queue(self):
+        return _MarshalFifoSerializationSQLiteQueue(self.qpath)
+
+
+class PickleFifoSQLiteQueueTest(t.FifoSQLiteQueueTest, FifoQueueTestMixin):
+    def queue(self):
+        return _PickleFifoSerializationSQLiteQueue(self.qpath)
+
+
+class MarshalLifoSQLiteQueueTest(t.LifoSQLiteQueueTest, LifoQueueTestMixin):
+    def queue(self):
+        return _MarshalLifoSerializationSQLiteQueue(self.qpath)
+
+
+class PickleLifoSQLiteQueueTest(t.LifoSQLiteQueueTest, LifoQueueTestMixin):
+    def queue(self):
+        return _PickleLifoSerializationSQLiteQueue(self.qpath)

tests/test_squeues_request.py

@@ -5,7 +5,7 @@ Queues that handle requests
 from __future__ import annotations
 
 from abc import ABC, abstractmethod
-from typing import TYPE_CHECKING
+from typing import TYPE_CHECKING, Any
 
 import pytest
 import queuelib
@@ -16,14 +16,20 @@ from scrapy.squeues import (
     FifoMemoryQueue,
     LifoMemoryQueue,
     MarshalFifoDiskQueue,
+    MarshalFifoSQLiteQueue,
     MarshalLifoDiskQueue,
+    MarshalLifoSQLiteQueue,
     PickleFifoDiskQueue,
+    PickleFifoSQLiteQueue,
     PickleLifoDiskQueue,
+    PickleLifoSQLiteQueue,
 )
 from scrapy.utils.misc import build_from_crawler
 from scrapy.utils.test import get_crawler
 
 if TYPE_CHECKING:
+    from pathlib import Path
+
     from scrapy.crawler import Crawler
 
 
@@ -166,6 +172,92 @@ class TestMarshalLifoDiskQueueRequest(TestRequestQueueBase):
             queue.close()
 
 
+class TestPickleFifoSQLiteQueueRequest(TestRequestQueueBase):
+    is_fifo = True
+
+    @pytest.fixture
+    def q(self, crawler, tmp_path):
+        queue = PickleFifoSQLiteQueue.from_crawler(
+            crawler=crawler, key=str(tmp_path / "pickle" / "fifo")
+        )
+        try:
+            yield queue
+        finally:
+            queue.close()
+
+
+class TestPickleLifoSQLiteQueueRequest(TestRequestQueueBase):
+    is_fifo = False
+
+    @pytest.fixture
+    def q(self, crawler, tmp_path):
+        queue = PickleLifoSQLiteQueue.from_crawler(
+            crawler=crawler, key=str(tmp_path / "pickle" / "lifo")
+        )
+        try:
+            yield queue
+        finally:
+            queue.close()
+
+
+class TestMarshalFifoSQLiteQueueRequest(TestRequestQueueBase):
+    is_fifo = True
+
+    @pytest.fixture
+    def q(self, crawler, tmp_path):
+        queue = MarshalFifoSQLiteQueue.from_crawler(
+            crawler=crawler, key=str(tmp_path / "marshal" / "fifo")
+        )
+        try:
+            yield queue
+        finally:
+            queue.close()
+
+
+class TestMarshalLifoSQLiteQueueRequest(TestRequestQueueBase):
+    is_fifo = False
+
+    @pytest.fixture
+    def q(self, crawler, tmp_path):
+        queue = MarshalLifoSQLiteQueue.from_crawler(
+            crawler=crawler, key=str(tmp_path / "marshal" / "lifo")
+        )
+        try:
+            yield queue
+        finally:
+            queue.close()
+
+
+@pytest.mark.parametrize(
+    "queue_cls",
+    [
+        PickleFifoSQLiteQueue,
+        PickleLifoSQLiteQueue,
+        MarshalFifoSQLiteQueue,
+        MarshalLifoSQLiteQueue,
+    ],
+)
+def test_sqlite_queue_survives_unclean_shutdown(
+    crawler: Crawler, tmp_path: Path, queue_cls: Any
+) -> None:
+    key = str(tmp_path / "queue")
+    queue = queue_cls.from_crawler(crawler=crawler, key=key)
+    queue.push(Request("https://toscrape.com"))
+    # No close(), as if the process had been killed. The connection is closed
+    # directly instead, since on Windows the file cannot be removed later while
+    # it is still open.
+    queue._db.close()
+
+    queue = queue_cls.from_crawler(crawler=crawler, key=key)
+    try:
+        assert len(queue) == 1
+        request = queue.pop()
+        assert request is not None
+        assert request.url == "https://toscrape.com"
+    finally:
+        queue.close()
+
+
 class TestFifoMemoryQueueRequest(TestRequestQueueBase):
     is_fifo = True