Make the stats_valid invariant hold across processes and transactions

The gate roed314#141 added was decided by a Python attribute, and only the row-level
write paths maintained the flag it read, so the invariant it advertised did not
hold: a second process kept serving what it had cached, and a replacement path
could swap new data under old caches without clearing anything.

Database-authoritative, and atomic.  quick_count, quick_count_distinct and
_quick_statistic now carry

    AND EXISTS (SELECT 1 FROM meta_tables WHERE name = %s AND stats_valid)

into the SELECT that reads the cached row, rather than consulting
self.table._stats_valid.  That attribute is a copy taken when the table object
was built: another process's restat=False write moves the row and not the copy,
which is precisely the deployment this is for, and a rolled-back transaction
moves the copy and not the row.  One statement rather than a flag query
followed by a cache query, because two statements are two snapshots and a write
can commit between them.  count() on an empty query likewise answers from
meta_tables.total, a single-row metadata lookup rather than the stale copy;
with a suffix it reads the suffixed counts row instead, since the maintained
total describes the live table.

_break_stats and _restore_stats now issue their UPDATE unconditionally, through
a shared _set_stats_valid: a transition skipped because the local copy already
said so is a transition skipped on the strength of a value that may describe
neither the database nor the present.  That UPDATE's row lock is also what
orders a refresh against writers, so a live refresh_stats now claims the row at
the start instead of only restoring it at the end.  Overlapping work is then
serialized into one of the two acceptable outcomes -- the write is in the data
the refresh reads, or its invalidation lands after the refresh commits --
rather than a write committing mid-rebuild and having its invalidation
overwritten.

Every replacement path makes the transition, with the rename.  update_from_file
(both forms), rewrite, reload, reload_all, reload_revert and the staged swaps
could all change the live data while leaving the flag true.  _swap_in_tmp and
reload_final_swap now take the intended state and write it inside the
transaction that does the renames, so the flag and the relations cannot come
apart; _reload_stats_valid states the rule for reload once, since reload_all
defers its swaps to a second pass.  A metafile's own stats_valid is overruled
by what the swap actually did -- the file describes the table it was exported
from, and cannot know whether these relations were rebuilt.  reload_revert
invalidates, because a backup carries no validity bit of its own, and recounts
the total for the same reason.

Cache maintenance asks a different question.  _record_count,
_record_count_distinct and _record_statistic chose between INSERT and
UPDATE/DELETE by calling the public lookups, which under the new gate report a
miss for a row that is physically present -- on every invalid table, which is
every table they run on.  Each recorder was leaving a second row under a key
the rest of the code takes to identify at most one; the plainest case was a
write clearing the flag and then duplicating the {} total row it maintains.
They now use private physical lookups (_cached_count, _cached_count_distinct,
_cached_statistic) that ignore the flag, and _record_statistic updates in place
rather than always inserting.  Exercising that branch for the first time also
turned up _record_count_distinct's update naming a column stats that has always
been called stat.

tests/test_stats_validity.py grows 30 cases: two-handle staleness for counts,
distinct counts and max/min/sum and for the total; a stale local false not
skipping an invalidation; a rolled-back restore; a threaded refresh/write race;
one per replacement path in both directions, the metafile interaction, and a
swap failing between the rename and the flag; and physical-row counts after
each recorder.  Every one of them fails on 798fdd9.

Full suite: 1342 passed, 35 skipped, 1 xfailed.  Ruff and the -W docs build
clean.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>

Author: roed314

CHANGELOG.md

@@ -370,29 +370,66 @@ hardening standalone use; the highlights:
   flag but read paths ignored it, so a count cached before a `restat=False`
   write kept being served afterwards -- verified: a query counted at 67, then
   every matching row changed, still answered 67. Every lookup that would serve
-  a cached answer now goes through one predicate and reports a miss while the
-  flag is false: `quick_count`, `quick_count_distinct` and `_quick_statistic`,
-  which is what makes `count`, `max`, `min` and `sum` compute the answer
-  instead of returning a recorded one. The line is whether a miss costs one
-  bounded query or a rebuild, so these are deliberately not gated: the
-  empty-query `total`, maintained on every write and so exact; the `_status` /
-  `status` / `extra_counts` inventory, which is how `refresh_stats` discovers
-  what to recompute; `_has_stats` / `_has_numstats`, which decide whether a
-  whole statistics family needs computing; and `null_counts`, whose fallback
-  is one full count *per search column*. Gating that last group made
-  `column_counts`, `numstats` and `null_counts` rebuild on every call with
-  nothing to converge on, since only `refresh_stats` restores the flag --
-  measured on the LMFDB, four minutes of downstream suite became over
-  forty-five. **The gap that leaves:** `column_counts`, `numstats` and
-  `null_counts` can still report a value recorded before an unrefreshed
-  write.
+  a cached answer now reports a miss while the flag is false: `quick_count`,
+  `quick_count_distinct` and `_quick_statistic`, which is what makes `count`,
+  `max`, `min` and `sum` compute the answer instead of returning a recorded
+  one. The line is whether a miss costs one bounded query or a rebuild, so
+  these are deliberately not gated: the empty-query `total`, maintained on
+  every write and so exact; the `_status` / `status` / `extra_counts`
+  inventory, which is how `refresh_stats` discovers what to recompute;
+  `_has_stats` / `_has_numstats`, which decide whether a whole statistics
+  family needs computing; and `null_counts`, whose fallback is one full count
+  *per search column*. Gating that last group made `column_counts`, `numstats`
+  and `null_counts` rebuild on every call with nothing to converge on, since
+  only `refresh_stats` restores the flag -- measured on the LMFDB, four minutes
+  of downstream suite became over forty-five. **The gap that leaves:**
+  `column_counts`, `numstats` and `null_counts` can still report a value
+  recorded before an unrefreshed write.
   Closing it needs freshness per statistic rather than one flag per table,
   which is a metadata format change; `refresh_stats()` is the remedy
   meanwhile. A suffixed (`_tmp`, `_oldN`) table is not gated by the live
-  table's flag, since it carries its own caches. The flag is restored only by
-  `refresh_stats()`, inside the transaction that rebuilt the caches, so a
-  failed refresh leaves the table invalid; refreshing a `_tmp` copy does not
-  validate the live table.
+  table's flag, since it carries its own caches.
+- **The flag is the database's, and it is tested in the same statement as the
+  cache.** Each gated lookup carries `AND EXISTS (SELECT 1 FROM meta_tables
+  WHERE name = %s AND stats_valid)` into the `SELECT` that reads the cached
+  row, rather than consulting the `_stats_valid` attribute a table object was
+  built with. That attribute is a copy: another process's `restat=False` write
+  moves the row and not the copy, so a second webserver process would have gone
+  on serving the counts it had cached, and a rolled-back transaction moves the
+  copy and not the row. Testing the flag in one statement and reading the cache
+  in the next would leave a window between two snapshots for a write to commit
+  in, so both readings go into one statement. `count()` on an empty query
+  likewise answers from `meta_tables.total` rather than from `self.total` --
+  a single-row metadata lookup, not a scan -- so a total another process
+  changed is the one served. `_break_stats` and `_restore_stats` now issue
+  their `UPDATE` unconditionally, since a transition skipped because the local
+  copy already said so is a transition skipped on stale information; the row
+  lock that `UPDATE` takes is also what serializes a refresh against concurrent
+  writers, a refresh now claiming the row before it rebuilds anything rather
+  than only restoring it at the end.
+- **Every replacement and bulk path makes its validity transition, in the
+  transaction that does the work.** `update_from_file` (in place or not),
+  `rewrite`, `reload`, `reload_all`, `reload_revert` and the staged swaps could
+  all change the live data while leaving `stats_valid = true`, so the new gate
+  went on serving the old counts. `_swap_in_tmp` and `reload_final_swap` now
+  take the intended state as an argument and write it with the renames:
+  true when the counts and stats arriving at the live names were rebuilt or
+  loaded for the data arriving with them, false when the old cache companions
+  are kept. A `metafile`'s own `stats_valid` is overruled by what the swap
+  actually did, `reload_revert` invalidates (a backup carries no validity bit
+  of its own) and also recounts the total, and nothing inherits the old live
+  table's flag.
+- **Cache maintenance asks whether the row exists, not whether it may be
+  served.** `_record_count`, `_record_count_distinct` and `_record_statistic`
+  chose between `INSERT` and `UPDATE`/`DELETE` by calling the public lookups,
+  which under the new gate answer "missing" for a row that is physically there
+  -- on every invalid table, which is every table they run on. Each write left
+  a second row behind under a key the rest of the code takes to identify at
+  most one; the plainest case was a write clearing the flag and then
+  duplicating the `{}` total row it maintains. They now use private physical
+  lookups that ignore the flag. This is also the first thing ever to reach
+  `_record_count_distinct`'s update statement, which named a column `stats`
+  that has always been called `stat`.
 - **Bulk paths run `ANALYZE`.** A relation that has just been bulk loaded has
   no planner statistics until autovacuum reaches it, so queries against it are
   costed as though it were tiny. Replacement tables are analyzed while still

DataManagement.md

@@ -110,12 +110,28 @@ These mutate the live table directly.  They are convenient for small edits; for
 
 **Statistics invalidation.**  Any write that can change the data calls `_break_stats`, which sets `meta_tables.stats_valid = false` so that cached statistics are known to be stale.  If the table has `saving` on and you left `restat=True`, statistics are refreshed at the end of the call; otherwise they are simply marked invalid.
 
+Inserting rows (and updating a sort-key column) also calls `_break_order`, setting `out_of_order = true` to record that the `id` order no longer matches `sort`; `delete` leaves the order flag alone.
+
+### What `stats_valid` promises
+
 `stats_valid` is enforced rather than merely recorded: while it is false, a
 cached nonempty-query count, distinct count, minimum, maximum or sum reports a
 miss, and the method computes the answer instead of returning the stored one.
 The empty-query `total` is the exception, since it is maintained on every write
 and so stays exact.
 
+The promise is made by the database, not by the process making it.  Each of
+those lookups tests `meta_tables.stats_valid` **in the same `SELECT`** that
+reads the cached row, so a cached answer can only be served under a snapshot
+that says the cache is valid; a table object's `_stats_valid` attribute is
+advisory, and correctness never rests on it.  That is what makes the flag hold
+in a deployment running several webserver processes: one process's
+`restat=False` write stops every other process from serving the affected
+counts, without any of them being told.  For the same reason `count()` on an
+empty query answers from `meta_tables.total` rather than from the copy its
+table object was built with, which is a single-row metadata lookup and not a
+scan.
+
 `column_counts`, `numstats` and `null_counts` are the other exception, and a
 caveat worth knowing.  The line is what a cache miss costs: the counts above
 fall back to a single statement about the rows in question, while these fall
@@ -124,16 +140,56 @@ column.  Making them miss while the table is invalid would rebuild on every
 call and never converge, since only `refresh_stats()` restores the flag, so
 they read what is recorded.  A value recorded before an unrefreshed write is
 therefore still reported by them; run `refresh_stats()` after a write you did
-not `restat`.  The flag goes back to true only in `refresh_stats()`, inside
-the transaction that rebuilt the caches, so a refresh that fails part-way leaves
-the table marked invalid rather than claiming a cache it does not have.
-Refreshing a `_tmp` copy does not validate the live table.
+not `restat`.
+
+The flag goes back to true only in `refresh_stats()`, inside the transaction
+that rebuilt the caches, so a refresh that fails part-way leaves the table
+marked invalid rather than claiming a cache it does not have; a rollback
+likewise leaves the stored flag false whatever the Python object was left
+saying.  Refreshing a `_tmp` copy does not validate the live table.
+
+A live `refresh_stats()` claims the table's `meta_tables` row at the start, by
+marking it invalid, and holds that row lock for the whole rebuild.  Every
+library write marks the same row, so a write that overlaps a refresh waits for
+it, and the result is one of the two orderings rather than a race: either the
+write went first and its rows are in the caches the refresh commits, or the
+refresh went first and the write's invalidation lands after it, leaving the
+table invalid.
+
+### Which operations set it, and to what
+
+Every write and swap makes its validity transition **in the same transaction as
+the data change or rename**, so the flag and the relations cannot come apart:
+
+| operation | leaves `stats_valid` |
+| --- | --- |
+| `insert_many`, `upsert`, `update`, `delete`, `copy_from`, in-place `update_from_file` | false, then true if `saving` and `restat` refreshed the caches |
+| non-inplace `update_from_file`, `rewrite` | true iff `saving` and `restat` (which is exactly when the rebuilt `_tmp` counts and stats are swapped in with the data) |
+| `reload`, `reload_all` | true iff `saving` and either `restat`, or both a `countsfile` and a `statsfile` were supplied |
+| staged commit, `staged_force_swap` | false — the staged counts and stats tables are empty, not refreshed |
+| `reload_revert` | false |
+| `refresh_stats()` on the live table | true |
+
+`reload_final_swap` and `_swap_in_tmp` take the intended state as a
+`stats_valid=` argument, defaulting to false; a deferred final swap (`reload`
+with `final_swap=False`) should be given the value its `reload` would have
+computed.  Nothing inherits the old live table's flag, which was a fact about
+data that is no longer there.
+
+Two consequences worth spelling out.  A `metafile` carries a `stats_valid`
+column, and the swap's own answer overrules it: the file records what was true
+of the table it was exported from, at export time, and cannot know whether the
+relations being swapped in were rebuilt.  And `reload_revert` clears the flag
+because a backup carries no validity bit of its own — `meta_tables` has one row
+and it stayed with the live name — so a backup taken while the table was
+invalid could otherwise come back under a flag that had since been set true; it
+also recounts the total for the same reason.
 
 Bulk paths also run PostgreSQL's own `ANALYZE`, which is a different thing from
 psycodict's statistics: a freshly loaded relation has no planner statistics
 until autovacuum reaches it.  Replacement tables are analyzed while still named
 `_tmp`, before the swap, since the catalog entry follows the relation through
-the rename; `copy_from` analyzes the live table it loaded into.  Inserting rows (and updating a sort-key column) also calls `_break_order`, setting `out_of_order = true` to record that the `id` order no longer matches `sort`; `delete` leaves the order flag alone.
+the rename; `copy_from` analyzes the live table it loaded into.
 
 ### Resorting is disabled
 

psycodict/database.py

@@ -2268,7 +2268,17 @@ def reload_all(
             for table, filedata, included in file_list:
                 if table in failures:
                     continue
-                table.reload_final_swap(tables=included, metafile=filedata[-1], sep=sep)
+                # The swaps are deferred to this second pass, so each one has
+                # to be told the validity its own reload established -- the
+                # call that knew the files is long over, and reload_final_swap
+                # invalidates by default.
+                countsfile, statsfile = filedata[1:3]
+                table.reload_final_swap(
+                    tables=included,
+                    metafile=filedata[-1],
+                    sep=sep,
+                    stats_valid=table._reload_stats_valid(countsfile, statsfile, restat),
+                )
 
         if failures:
             print("Reloaded %s" % (", ".join(tablenames)))