Retires only per-host chart hashes in the conformance suite

changelog.d/sp-gbyk.md

@@ -0,0 +1,3 @@
+### Fixed
+
+- `StatifierPersistence.Testing.StorageConformance` retires only chart hashes derived from the using module's name, so two or more modules running it asynchronously against one Postgres database no longer deadlock (`40P01`) in the tombstone-check case or in the cases that retire a hash after a create's or a migration's first check.

docs/adr/0012-retention-and-retirement.md

@@ -810,3 +810,50 @@ What was re-read before the flip:
   `{:chart_retired, info}` answer.
 - **The changelog.** The 0.23.0 section of `CHANGELOG.md` names the race
   under Changed and the two conformance cases under Added.
+
+## Note (2026-09-29, sp-gbyk): a caller that reads a hash's tombstone and then retires that hash in one transaction upgrades the hash's lock, and the lock's key is database-wide
+
+This Note decides nothing; it states a hazard the sp-3v2c Amendment's
+lock carries, and what the conformance suite now does about it. The
+`lib/` cites were read on `main` at `b11af5d`, except the suite's, which
+the change that carries this Note edits.
+
+**The upgrade.** On the Ecto adapter over Postgres a tombstone read
+takes the hash's shared lock (`lib/statifier_persistence/storage/ecto.ex`,
+`fetch_retired_info/2`), and a retirement takes the exclusive one
+(`retire_chart/3`). Both locks are transaction-scoped. A caller whose
+own transaction first reads a hash's tombstone, or creates or migrates
+an execution onto that hash, and then retires the same hash asks for
+the exclusive lock while it still holds the shared one. One such
+transaction alone is granted the upgrade. Two such transactions on one
+hash each hold the shared lock and each wait for the other to release
+it, and Postgres ends one of them with `deadlock_detected` (SQLSTATE
+`40P01`).
+
+**No path the package offers does this on its own.** `retire_chart/3`
+takes the exclusive lock as the first statement of its transaction and
+holds nothing on the hash before it; `create/4`, `migrate/4`,
+`migrate_tree/4` and `StatifierPersistence.Storage.check_chart_retired/2`
+take only the shared lock; and
+`StatifierPersistence.Executions.retire_chart/4` asks for the active
+executions and the pin sources' counts before the retirement, with no
+tombstone read. The upgrade is reached only when a caller composes a
+tombstone read and a retirement of one hash inside a transaction of
+its own. A test that runs in a sandbox transaction lasting the whole
+test, and reads a hash's tombstone before it retires that hash, is such
+a caller.
+
+**The key is database-wide.** The lock's key is
+`(@chart_lock_namespace, hashtext(content_hash))` (`chart_lock/3`). No
+table, table prefix or schema is part of it, so two hosts, or two
+suites, that store charts in different tables of one database still
+share the lock of every hash they have in common.
+
+**What the conformance suite does.** Its three Ecto modules in this
+package ran the same suite asynchronously against one database, and the
+cases that read a tombstone and then retire its hash met on one key and
+deadlocked. Every chart hash a generated case retires now carries a
+suffix derived from the using module's name
+(`lib/statifier_persistence/testing/storage_conformance.ex`,
+`@conformance_hash_suffix`), so two hosts running the suite in one
+database never retire the same hash.

lib/statifier_persistence/testing/storage_conformance.ex

@@ -147,6 +147,22 @@ defmodule StatifierPersistence.Testing.StorageConformance do
       @conformance_adapter_opts conformance_adapter_opts
       @conformance_prune_scope conformance_prune_scope
 
+      # Every chart hash a generated case retires is this host module's
+      # own. On Postgres a retirement takes the hash's advisory lock
+      # exclusive, the tombstone read takes it shared, and the key is
+      # database-wide: no table, prefix or schema is in it. Cases in one
+      # module run one at a time, but two async modules running this
+      # suite against one database run the same case at once, each in a
+      # sandbox transaction that lasts the whole test; with one shared
+      # hash, a case that reads a tombstone and then retires the hash
+      # upgrades its shared lock while the other module's copy holds
+      # the same shared lock, and Postgres answers 40P01. A suffix
+      # derived from the module name keeps each host on keys of its own.
+      @conformance_hash_suffix :sha256
+                               |> :crypto.hash(inspect(__MODULE__))
+                               |> Base.encode16(case: :lower)
+                               |> binary_part(0, 16)
+
       setup do
         {:ok, store} = Storage.new(@conformance_adapter, @conformance_adapter_opts)
 
@@ -1274,7 +1290,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
 
       if Code.ensure_loaded?(conformance_adapter) and
            function_exported?(conformance_adapter, :retire_chart, 3) do
-        @retire_hash "sha256:conformance-retire"
+        @retire_hash "sha256:conformance-retire-" <> @conformance_hash_suffix
 
         # sabotage: drop the adapter under test's guard on the :active
         # arm - the `Adapter.pinned?(counts) -> ...` cond clause in the
@@ -1608,7 +1624,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
           assert {:error, :chart_not_found} =
                    @conformance_adapter.retire_chart(
                      store.opts,
-                     "sha256:conformance-retire-never-stored",
+                     "sha256:conformance-retire-never-stored-" <> @conformance_hash_suffix,
                      retirement()
                    )
         end
@@ -1696,6 +1712,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
                 <transition event="book.returned" target="returned"/>
             </state>
             <final id="returned"/>
+            <!-- conformance host #{@conformance_hash_suffix} -->
         </scxml>
         """
 
@@ -1706,6 +1723,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
                 <transition event="book.returned" target="returned"/>
             </state>
             <final id="returned"/>
+            <!-- conformance host #{@conformance_hash_suffix} -->
         </scxml>
         """
 
@@ -2090,7 +2108,9 @@ defmodule StatifierPersistence.Testing.StorageConformance do
       # Reverted from a copy.
       test "facade: a retirement either runs or is declined at open", %{store: store} do
         answer =
-          Storage.retire_chart(store, "sha256:conformance-retire-capability",
+          Storage.retire_chart(
+            store,
+            "sha256:conformance-retire-capability-" <> @conformance_hash_suffix,
             retired_by: "conformance-operator"
           )
 
@@ -2150,7 +2170,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
           # from a copy.
           test "adapter: the tombstone read answers what the retired arm of fetch_chart/2 carries",
                %{store: store} do
-            hash = "sha256:conformance-tombstone-retired"
+            hash = "sha256:conformance-tombstone-retired-" <> @conformance_hash_suffix
             at = DateTime.from_naive!(~N[2026-09-23 09:00:00.000000], "Etc/UTC")
 
             assert :ok =
@@ -3234,7 +3254,7 @@ defmodule StatifierPersistence.Testing.StorageConformance do
       # reverted from a copy.
       test "facade: the tombstone check refuses a retired chart and lets a live one through",
            %{store: store} do
-        {source, machine} = Charts.chart_a()
+        {source, machine} = own_chart_a()
         content_hash = Machine.identity(machine).content_hash
 
         assert :ok = Storage.check_chart_retired(store, machine)
@@ -3304,6 +3324,24 @@ defmodule StatifierPersistence.Testing.StorageConformance do
         if self() == pid, do: send(pid, {:adapter_call, metadata.callback})
       end
 
+      # Chart "a" with a comment naming this host module's hash suffix,
+      # so its content hash is this module's own: the case above
+      # retires it, and a retirement must never meet another async
+      # module's copy of the case on one lock key.
+      defp own_chart_a do
+        {source, _machine} = Charts.chart_a()
+
+        own_source =
+          String.replace(
+            source,
+            "</scxml>",
+            "    <!-- conformance host #{@conformance_hash_suffix} -->\n</scxml>"
+          )
+
+        {:ok, machine} = Statifier.compile(own_source)
+        {own_source, machine}
+      end
+
       defp narrow_read_declared?(store) do
         if function_exported?(@conformance_adapter, :supports_retired_info?, 1) and
              function_exported?(@conformance_adapter, :fetch_retired_info, 2) do