improvement: allow write transactions per resource via write_transactions?

[C-Sinclair]

Closes the implementation half of #91. Supersedes #95, which started this and was closed as stale.

What

can?(:transact) has returned false since this data layer was split out of ash_postgres. The transactions guide explains why: SQLite allows one write lock at a time, and a write attempted while another transaction holds that lock fails immediately rather than queueing.

That is a reason to make transactions opt in, not a reason to leave them unimplemented. SQLite is fully ACID, and without a transaction a create whose after_action hook fails leaves its row behind with nothing to undo it.

This adds write_transactions? to the sqlite section, defaulting to false so nothing changes for existing resources:

sqlite do
  table "accounts"
  repo MyApp.Repo
  write_transactions? true
end

Why this shape

#91 settled on a per-resource opt-in with this name, after discussion between @jimsynz and @zachdaniel. This follows that.

It composes with functional repos rather than replacing them. The read/write pool split discussed in #91 is repo fn _, :mutate -> WriteRepo; _, :read -> ReadRepo end plus this flag on the write side, and the guide documents the pair.

The callbacks

• transaction/4 opens write transactions as BEGIN IMMEDIATE. This part is really important. A deferred transaction takes no lock until its first write, so a read-then-write has to upgrade partway through. And SQLite cannot make an upgrade wait for busy_timeout, because the snapshot it already read from may be stale by the time the lock frees. It fails immediately regardless of the timeout. BEGIN IMMEDIATE has nothing to upgrade. Read-only transactions stay deferred, since they never take the write lock.

  This holds regardless of the repo's default_transaction_mode, so it works without asking users to configure the adapter correctly first.

• in_transaction?/1 answers rather than raising when the repo was reached through put_dynamic_repo/1 and started under no name of its own. Ecto.Repo.in_transaction?/0 resolves the current dynamic repo through the registry and raises when it is absent; Ash asks this before opening a transaction, so it needs an answer. No running repo means no open transaction.

• prefer_transaction_for_atomic_updates?/1 is false. An atomic update is a single statement and so already atomic; wrapping it would hold the one write lock across the surrounding work and buy nothing.

There is deliberately no verifier

#91's proposal 3 and #95 both included a verifier rejecting transaction? true on a resource that has not opted in. It cannot work, because there is nothing there to check.

transaction? is not something an author opts into — it is the struct default for every create, update and destroy action (Ash.Resource.Actions.Create, .Update, .Destroy all set transaction?: true). So a mutation action carries true unless somebody explicitly turned it off, and an action written by an author who never thought about transactions is indistinguishable from one that asked for them. (Derivation compounds this — an action using manage_relationship gets the flag too — but the default alone is enough.) A verifier therefore fires on essentially every resource in the ecosystem: compiled against this repository's own suite it flagged four existing test resources — Comment, Device, Manager, Post — none of which mentions transaction? at all.

ash-project/ash#2895 makes the verifier unnecessary as well as unworkable. Ash.Resource.Transformers.SetActionTransactions now clears the flag on a resource whose data layer cannot transact, so the DSL says what will actually happen instead of naming a transaction that never opens. Since Spark runs every transformer before any verifier, that also closes off the last escape hatch: explicit and defaulted transaction? true are both already false by the time a verifier could look.

This branch requires ash >= 3.32.1 for it, and test/transaction_test.exs pins the behaviour: Ash.Resource.Info.action(Account, :create).transaction? is false, and true on TransactionalAccount. Discoverability is handled in the guide.

Note

mix test.check_migrations reports 3 pending files on main as well as on this branch — pre-existing, unrelated to this change.

[zachdaniel]

@C-Sinclair I think we can fix this in Ash core by only setting transaction? true when the data layer supports it instead of always. Since up til now its just been ignored when test to true.

[zachdaniel]

Like the bit about not supporting the verifier.

[zachdaniel]

Latest version of ash is released, so you can update the dependency on Ash and leverage that transformer change 🙇

[zachdaniel]

Having more thoughts on this: given that the place that you configure this is actually on the repo by configuring a busy wait to a high enough value (equal to or higher than pool config allows running a query?) then I think we should make this configuration also live on the repo). That way you don't need to configure this all across your repos, and in practice it's a repo/connection-pool level configuration. Then can(:transact) can look at the repo module to determine if the resource can do so.

Thoughts?