Fix concurrency, lifecycle and contract problems from the second review

Schema watcher:
- Scan coalescing is loop-agnostic: in-flight scans are concurrent.futures
  futures any event loop can wait for, taken over only when their loop has
  closed or stopped; each refresh() resolves only its own scans. A write on
  one loop no longer hangs forever when another loop's scan is in flight.
- At most one polling task across event loops; replaced pollers exit.
- A scan or catalog write that fails transiently (internal database locked,
  shared-cache lock, cancellation) re-marks the database and retries it from
  later requests and sweeps, with back-off; requests are never failed by it.
  Write threads mark schema changes directly (under a lock) instead of
  relying on a notification reaching a loop that may have closed.
- add_database() file databases and named in-memory databases use the
  default_schema_watch mode (external unless configured). Named in-memory
  databases can be external: polled with PRAGMA schema_version on their
  write connection. They are not scanned at registration time, which raced
  plugins filling them (datasette-copy-to-memory).
- close() no longer persists a fingerprint for an owned database whose file
  header shows a schema the catalog has not seen.
- The derived-table permission rule (external-content FTS) trusts the
  catalog only when a stat and header read prove it is current; otherwise
  it asks the database, and schedules a rescan if the catalog is behind.

Connections and write threads:
- A BaseException from a write or isolated callback is delivered to the
  caller as a RuntimeError instead of killing the thread silently.
- conn.close() on a write lease raises ConnectionLeaseError, and the write
  thread reopens a connection something else closed.
- Write connections pin PRAGMA recursive_triggers=on (sqlite-utils FTS
  triggers rely on it; idle reopen lost the old drift).
- execute_fn() accepts generators that no longer hold the connection or a
  cursor.
- --crossdb _memory connections count against max_open_connections, are
  reaped, and are discarded when databases are added or removed.

Lifecycle:
- The temp internal database file uses weakref.finalize, not atexit, so
  closed or abandoned instances can be garbage collected.
- The read-pool reaper no longer keeps the last pool alive in its frame.
- The scratch directory is loaded (and locked) at the end of __init__, so a
  failed construction leaves nothing locked.
- config_dir/scratch is used only if empty or created by Datasette; a second
  instance on the same config dir starts without it; sidecar files are never
  deleted the first time a directory is opened.

Regression tests: tests/test_connection_review_fixes.py (24 of 27 fail on
a509b81; the other 3 guard behaviour that must not change).

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01EwkLo2rJWidpkDVT3HkvNv
This commit is contained in:
Claude 2026-10-04 08:32:09 +00:00
commit d7a2ce91cc
No known key found for this signature in database
10 changed files with 1413 additions and 153 deletions

View file

@ -262,9 +262,10 @@ These can be passed to ``datasette serve`` using ``datasette serve --setting nam
schema_watch_interval_ms How often to check external database files for
schema changes - 0 to disable polling
(default=1000)
default_schema_watch Schema watch mode for databases opened from
files: external (poll for changes made by other
processes) or owned (only Datasette changes them)
default_schema_watch Schema watch mode for database files and named
in-memory databases without a mode of their own:
external (poll for changes made outside
Datasette) or owned (only Datasette changes them)
(default=external)
sql_time_limit_ms Time limit for a SQL query in milliseconds
(default=1000)

View file

@ -1328,7 +1328,7 @@ Removes the column type assignment for the specified column.
This will be used in the URL path. If not specified, it will default to the same thing as the ``name``.
``schema_watch`` - string, optional
How Datasette keeps its catalog of this database's tables current: ``"owned"`` (the default - only Datasette's own writes change the schema), ``"external"`` (other processes may change the file too, so it is polled) or ``"immutable"``. A ``schema_watch`` setting for this database in ``datasette.yaml`` takes precedence. See :ref:`setting_schema_watch_interval_ms`.
How Datasette keeps its catalog of this database's tables current: ``"owned"`` (only Datasette's own writes change the schema), ``"external"`` (other code may change it too - another process, or a plugin's own connection - so it is polled) or ``"immutable"``. The default for a database file or a named in-memory database is the :ref:`setting_default_schema_watch` setting, ``"external"`` unless configured. Pass ``"owned"`` if every change to the database goes through Datasette's write methods: it is then never polled. A ``schema_watch`` setting for this database in ``datasette.yaml`` takes precedence. See :ref:`setting_schema_watch_interval_ms`.
The ``datasette.add_database(db)`` method lets you add a new database to the current Datasette instance.
@ -1405,10 +1405,10 @@ A *scratch database* is a SQLite file that a plugin (or a user, through a plugin
They live in the **scratch directory**:
- ``datasette --scratch-dir path/to/scratch`` (or the ``DATASETTE_SCRATCH_DIR`` environment variable, or ``Datasette(scratch_dir=...)``). The directory is created if it does not exist.
- In :ref:`config_dir`, a ``scratch/`` directory inside the configuration directory is used if it exists.
- In :ref:`config_dir`, a ``scratch/`` directory inside the configuration directory is used if it exists and either is empty or was created by Datasette (it contains ``.datasette-scratch.db``). A ``scratch/`` folder of your own that holds other files is left alone, with a warning in the log. If another Datasette instance is already using it - a server, while you run ``datasette mydir --get ...`` - the second instance starts without its scratch databases, with a warning.
- Otherwise scratch databases are **temporary**: they go in a temporary directory that is created the first time a plugin creates one and deleted when Datasette shuts down, the same way the internal database is temporary unless ``--internal`` is used. ``datasette.scratch_dir`` is ``None`` until then. Plugins can use the same API either way.
Use a directory just for scratch databases. Only one Datasette instance can use a scratch directory at a time: a second one fails to start with an error.
Use a directory just for scratch databases. Only one Datasette instance can use a scratch directory at a time: a second one given the same ``--scratch-dir`` fails to start with an error. The first time Datasette opens a directory it adopts any SQLite files already in it, but never deletes anything: leftover ``-wal``, ``-shm`` or ``-journal`` files without their database are only removed once the directory holds Datasette's registry.
The directory contains one ``<name>.db`` file per scratch database, plus a registry (``.datasette-scratch.db``) that records each database's owner, metadata, creation time and when it was last used. Scratch databases are created in WAL mode (rollback journal mode with ``--nolock``).
@ -2233,7 +2233,9 @@ Connections are only valid inside your function
The ``conn`` passed to an ``execute_fn()``, ``execute_write_fn()`` or ``execute_isolated_fn()`` function - and to :ref:`plugin_hook_write_wrapper` hooks - is lent to that function for the duration of the call. It behaves like a ``sqlite3.Connection`` (``isinstance(conn, sqlite3.Connection)`` is ``True`` and it works with ``sqlite_utils.Database(conn)``), but once the function returns it stops working: any further use raises ``datasette.database.ConnectionLeaseError``. Do not store the connection on an object, in a global or in a closure for later use. Do everything you need inside the function and return plain Python values.
``execute_fn()`` also raises ``ConnectionLeaseError`` if your function returns a cursor or a generator, since those would keep reading from the connection after it had been returned to the pool.
``execute_fn()`` also raises ``ConnectionLeaseError`` if your function returns a cursor, or a generator that holds the connection or a cursor (for example ``(row for row in conn.execute(...))``), since those would keep reading from the connection after it had been returned to the pool. A generator over rows already fetched inside the function is fine.
The write connection belongs to Datasette: calling ``conn.close()`` inside an ``execute_write_fn()`` function raises ``ConnectionLeaseError``. Closing the connection passed to an ``execute_fn()`` or ``execute_isolated_fn()`` function is allowed.
Datasette cannot detect every misuse. These are unsupported and may fail unpredictably, or read from a connection that is now being used by someone else:
@ -2366,7 +2368,7 @@ Example usage:
version = await db.execute_fn(get_version)
The connection is borrowed from a pool for the duration of the call (see :ref:`database_connections`) and is only valid inside the function. Using it after the function has returned - for example by storing it on an object - raises a ``datasette.database.ConnectionLeaseError``. Returning a cursor or a generator from the function raises the same error: fetch the rows you need before returning. See :ref:`database_connection_leases`.
The connection is borrowed from a pool for the duration of the call (see :ref:`database_connections`) and is only valid inside the function. Using it after the function has returned - for example by storing it on an object - raises a ``datasette.database.ConnectionLeaseError``. Returning a cursor, or a generator that is still reading from the connection, raises the same error: fetch the rows you need before returning. See :ref:`database_connection_leases`.
Calls to ``execute_fn()`` are not guaranteed to use the same connection, so temporary tables, ``ATTACH`` statements and functions created inside one call may not be visible in the next.
@ -2455,7 +2457,9 @@ This method works like ``.execute_write()``, but instead of a SQL statement you
The function can then perform multiple actions, safe in the knowledge that it has exclusive access to the single writable connection for as long as it is executing.
The connection is only valid while your function is running: using it after the function has returned raises ``datasette.database.ConnectionLeaseError`` - see :ref:`database_connection_leases`. The write connection is closed after :ref:`setting_connection_idle_timeout_ms` without writes, so temporary tables, ``ATTACH`` statements and functions created by one write function may be gone by the next.
The connection is only valid while your function is running: using it after the function has returned raises ``datasette.database.ConnectionLeaseError`` - see :ref:`database_connection_leases`. The write connection is closed after :ref:`setting_connection_idle_timeout_ms` without writes, so temporary tables, ``ATTACH`` statements and functions created by one write function may be gone by the next. Every new write connection has ``PRAGMA recursive_triggers`` turned on (before any :ref:`plugin_hook_prepare_connection` hook runs), which is what ``sqlite-utils`` sets and what the triggers it creates to keep full-text search indexes current rely on for ``INSERT OR REPLACE``.
If your function raises an exception, ``execute_write_fn()`` raises it too and the transaction is rolled back. An exception that is not a subclass of ``Exception``, such as ``SystemExit``, is raised as a ``RuntimeError`` whose ``__cause__`` is the original exception.
Like ``execute_fn()``, the call is traced as a ``db.query`` OpenTelemetry span carrying ``datasette.callback`` rather than ``db.query.text``, above the write-queue spans - see :ref:`internals_telemetry`. A named function gives the span a readable identity; a lambda reports ``<lambda>``.

View file

@ -240,7 +240,7 @@ Set this to 0 for no limit, in which case idle connections are only closed by :r
datasette *.db --setting max_open_connections 64
The cap does not cover write connections (one per database that is being written to, see :ref:`setting_connection_idle_timeout_ms`), the short-lived connections used by :ref:`database_execute_isolated_fn` and by schema checks, or in-memory databases, which use no file descriptors.
The cap does not cover write connections (one per database that is being written to, see :ref:`setting_connection_idle_timeout_ms`), the short-lived connections used by :ref:`database_execute_isolated_fn` and by schema checks, or in-memory databases, which use no file descriptors. The exception is the ``_memory`` database under ``--crossdb``: each of its connections ``ATTACH``\ es up to ten database files, so those connections count against the cap and are closed when idle like file connections. Adding or removing a database closes them too, so the next cross-database query sees the current set of databases.
Each pooled connection to a file in WAL mode uses three file descriptors (the database, its ``-wal`` and its ``-shm`` file); other databases use one. Under a small file descriptor limit such as ``ulimit -n 256``, a cap of 64 keeps pooled read connections below about 200 descriptors even if every database uses WAL, leaving room for write connections and Datasette itself.
@ -298,7 +298,7 @@ Set this to ``0`` to turn polling off. External databases are then only checked
datasette mydatabase.db --setting schema_watch_interval_ms 0
Databases opened from files use the :ref:`setting_default_schema_watch` mode. Databases added by plugins with :ref:`datasette.add_database() <datasette_add_database>` are ``owned`` unless the plugin passes ``schema_watch="external"``. You can set the mode for an individual database in ``datasette.yaml``, which overrides both:
Database files, and named in-memory databases, use the :ref:`setting_default_schema_watch` mode - including those added by plugins with :ref:`datasette.add_database() <datasette_add_database>`, unless the plugin passes ``schema_watch="owned"`` (or ``"external"``). A named in-memory database in ``external`` mode is checked with ``PRAGMA schema_version`` on its write connection every ``schema_watch_interval_ms``, which notices tables created through a plugin's own connection to it. You can set the mode for an individual database in ``datasette.yaml``, which overrides both:
.. code-block:: yaml
@ -319,7 +319,7 @@ Default: ``external``
.. [[[end]]]
The schema watch mode, ``external`` or ``owned``, for mutable database files passed on the command line or with ``Datasette(files=...)`` - see :ref:`setting_schema_watch_interval_ms`.
The schema watch mode, ``external`` or ``owned``, for mutable database files and named in-memory databases that do not have a mode of their own: files passed on the command line or with ``Datasette(files=...)``, and databases added with :ref:`datasette.add_database() <datasette_add_database>` without a ``schema_watch`` argument - see :ref:`setting_schema_watch_interval_ms`.
The default, ``external``, suits the common workflow of changing a database with another tool such as ``sqlite-utils`` while Datasette is serving it: new tables and columns show up within about a second. If Datasette is the only thing that ever changes your database files, set this to ``owned`` and they will not be polled. This is worth doing for instances with thousands of databases, where polling costs a few milliseconds of CPU per second::