diff --git a/datasette/schema_watcher.py b/datasette/schema_watcher.py index ea7be2ea..6fa5ed7f 100644 --- a/datasette/schema_watcher.py +++ b/datasette/schema_watcher.py @@ -71,6 +71,9 @@ SCAN_CONCURRENCY = 1 # sweep with os.scandir() instead of probing -wal/-journal with stat() SCANDIR_THRESHOLD = 8 PRAGMA_BUSY_TIMEOUT_MS = 200 +# Read the schema cookie from the file header instead of opening a +# connection when no -wal/-journal has content +USE_HEADER_CHECK = True class WatchState: @@ -171,6 +174,35 @@ def is_racy(fp, t_ns): return newest_timestamp(fp) >= t_ns - RACY_WINDOW_NS +SQLITE_HEADER = b"SQLite format 3\x00" + + +def header_schema_version(path): + """The schema cookie (offset 40 of the database header) read straight + from the file - what PRAGMA schema_version returns - without opening a + SQLite connection or taking any lock. + + Only meaningful when there is no non-empty -wal or -journal: then the + main file is the whole database. The caller must have taken the + fingerprint *before* calling this; a commit in flight at that moment has + already bumped the mtime it saw, so the fingerprint is racy and the next + sweep looks again. + """ + try: + fd = os.open(path, os.O_RDONLY) + except OSError: + return None + try: + header = os.pread(fd, 100, 0) + except OSError: + return None + finally: + os.close(fd) + if len(header) < 100 or not header.startswith(SQLITE_HEADER): + return None + return int.from_bytes(header[40:44], "big") + + def _fp_to_json(fp, t_ns): return json.dumps({"fp": fp, "t": t_ns}) @@ -198,6 +230,7 @@ class SchemaWatcher: "sweep_seconds": 0.0, "stats": 0, "pragma_checks": 0, + "header_checks": 0, "scans": 0, "write_detections": 0, "replaced": 0, @@ -476,8 +509,10 @@ class SchemaWatcher: elif kind == "busy": self.counters["busy"] += 1 state.fp_racy = True # check again next sweep - elif kind == "checked": - self.counters["pragma_checks"] += 1 + elif kind in ("checked", "header"): + self.counters[ + "pragma_checks" if kind == "checked" else "header_checks" + ] += 1 state.stats["checks"] += 1 if version != state.catalog_version: state.needs_scan = True @@ -525,6 +560,14 @@ class SchemaWatcher: # first sight: the schema_version alone cannot be trusted out.append((state, "replaced", fp, t_ns, None)) continue + if USE_HEADER_CHECK and fp[1] is None and fp[2] is None: + # No -wal/-journal content: read the cookie from the + # header. No connection, no SHARED lock that could make + # a concurrent writer back off. + version = header_schema_version(os.fspath(state.db.path)) + if version is not None: + out.append((state, "header", fp, t_ns, version)) + continue out.append(self._pragma_check(state, fp, t_ns)) return out diff --git a/tests/test_schema_watcher.py b/tests/test_schema_watcher.py index 2f26f79d..2b876809 100644 --- a/tests/test_schema_watcher.py +++ b/tests/test_schema_watcher.py @@ -229,7 +229,9 @@ async def test_external_data_only_insert_does_not_rescan(db_path): @pytest.mark.asyncio -async def test_external_add_column(db_path): +@pytest.mark.parametrize("use_header", (True, False)) +async def test_external_add_column(db_path, monkeypatch, use_header): + monkeypatch.setattr(schema_watcher_module, "USE_HEADER_CHECK", use_header) ds = await start_ds([db_path]) run_external(db_path, "alter table t add column b text") @@ -395,11 +397,41 @@ def _coarse_fingerprint(real): return fingerprint +@pytest.mark.parametrize("journal_mode", ("delete", "wal", "truncate")) +def test_header_schema_version_matches_pragma(tmp_path, journal_mode): + path = str(tmp_path / "h.db") + conn = sqlite3.connect(path, isolation_level=None) + conn.execute(f"PRAGMA journal_mode={journal_mode}") + for i in range(300): + conn.execute(f"create table t{i} (id integer)") + conn.execute("alter table t1 add column b") + version = conn.execute("PRAGMA schema_version").fetchone()[0] + if journal_mode == "wal": + # Pages still in the -wal: the main file header is stale, which is + # why the watcher only reads it when the -wal is empty + assert schema_watcher_module.fingerprint(path)[1] is not None + conn.execute("PRAGMA wal_checkpoint(TRUNCATE)") + assert schema_watcher_module.fingerprint(path)[1:] == (None, None) + assert schema_watcher_module.header_schema_version(path) == version == 301 + conn.close() + assert ( + schema_watcher_module.header_schema_version(str(tmp_path / "nope.db")) is None + ) + (tmp_path / "junk.db").write_bytes(b"not a database" * 20) + assert ( + schema_watcher_module.header_schema_version(str(tmp_path / "junk.db")) is None + ) + + @pytest.mark.asyncio +@pytest.mark.parametrize("use_header", (True, False)) @pytest.mark.parametrize("racy_rule", (True, False)) -async def test_racily_clean_same_tick_change(db_path, monkeypatch, racy_rule): +async def test_racily_clean_same_tick_change( + db_path, monkeypatch, racy_rule, use_header +): """Two schema changes inside one timestamp tick, same file size and inode: only the racy-clean rule catches the second one.""" + monkeypatch.setattr(schema_watcher_module, "USE_HEADER_CHECK", use_header) monkeypatch.setattr( schema_watcher_module, "fingerprint",