improvement: manage tenant databases, so strategy :context works out of the box - #225
Open
C-Sinclair wants to merge 8 commits into
Open
improvement: manage tenant databases, so strategy :context works out of the box#225C-Sinclair wants to merge 8 commits into
strategy :context works out of the box#225C-Sinclair wants to merge 8 commits into
Conversation
…tions?`
`can?(:transact)` has returned false since this data layer was split out of
ash_postgres, and 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.
Adds `write_transactions?` to the `sqlite` section, defaulting to false so
nothing changes for existing resources, and implements the callbacks it enables:
* `transaction/4` opens write transactions as `BEGIN IMMEDIATE`. This is what
makes `busy_timeout` effective. 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, because the snapshot already read from
may be stale by the time the lock frees. It fails immediately regardless of
`busy_timeout`. `BEGIN IMMEDIATE` has nothing to upgrade. Read-only
transactions stay deferred, since they never take the write lock.
* `in_transaction?/1` answers rather than raising when the repo has no running
process. `Ecto.Repo.in_transaction?/0` resolves the current dynamic repo
through the registry and raises when it is absent, which happens whenever
the repo was reached through `put_dynamic_repo/1` and started under no name
of its own. Ash asks this before opening a transaction, so it has to be an
answer.
* `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.
Refs ash-project#91, and supersedes the work in ash-project#95.
Two notes on what is deliberately not here.
The option composes with functional repos rather than replacing them: the
read/write pool split discussed in ash-project#91 is `repo fn _, :mutate -> WriteRepo; _,
:read -> ReadRepo end` plus this flag on the write side, and the guide documents
the pair.
There is no verifier rejecting `transaction? true` on a resource that has not
opted in, which ash-project#91 proposed and ash-project#95 implemented. It cannot work as specified:
Ash *derives* `transaction? true` from the action rather than only taking it
from the author -- an action using `manage_relationship` gets it automatically --
so the check fires on resources nobody annotated. Compiling it against this
suite flags four existing test resources (Comment, Device, Manager, Post), none
of which mentions `transaction?` at all, and the error asks the author to change
an action they did not write. On a released version it would break existing
applications at compile time on upgrade. Discoverability is handled in the guide
instead.
…they get Ash now clears the derived `transaction? true` on a resource whose data layer cannot transact, so a resource without `write_transactions?` reflects `transaction? false` instead of naming a transaction that never opens.
…nder Closes the data layer half of ash-project#127. Turning on `strategy :context` currently fails at compile time with `Data layer does not support multitenancy`. The documented workaround -- setting `%{data_layer: %{repo: ...}}` in a change and a preparation -- does not work. Both the read path (`repo.all/2`) and the write path (`repo.insert_all/3`, via `AshSql.dynamic_repo/3`) invoke the result *as a module*, so passing an instance raises `ArgumentError: Modules (the first argument of apply) must always be an atom`. That override selects between repo *modules*; database-per-tenant needs an *instance*, and binding an instance is Ecto's job via `put_dynamic_repo/1`. ## What context multitenancy means for SQLite One database file per tenant. There is no schema to prefix, so the generated SQL is identical for every tenant and isolation comes from which file the connection is attached to. `set_tenant/3` is therefore a no-op on the query, and the tenant is deliberately not passed to `AshSql.repo_opts/5` -- it reached Ecto as a table prefix and raised `SQLite3 does not support table prefixes` on every write. ## The binder `tenant_binder` names a module implementing `AshSqlite.TenantBinder`, which is asked for a connection once per statement: sqlite do table "posts" repo MyApp.Repo tenant_binder MyApp.TenantBinder end This PR ships the seam and nothing behind it -- there is no default binder, and `strategy :context` without one is refused. A managed runtime that supplies one is a follow-up, so that this can be reviewed on its own. ## Why the data layer rather than the caller Every entry point would otherwise have to call `put_dynamic_repo/1` before Ash runs, and some cannot: - `Ash.count/2` never enters `Ash.Actions.Read`, so no preparation or `around_transaction` hook runs for it. - Ash calls `atomic/3` rather than `change/3` whenever it can build one statement, so a hook-installing change forces `require_atomic? false`. - The binding is ambient, so it does not survive `Task.async`, an `Ash.load` fan-out, or a background job. `bind/3` also receives the resource and a `usage` of `:read`, `:write` or `:transaction`. Only the data layer can say which, and it is what lets a binder serve reads from a replica while writes go to the owner. ## Interaction with transactions `transaction/4` never receives the tenant: Ash calls it above the data layer and the reason it builds does not name one. `AshSqlite.Transformers.CarryTenant` adds a change that puts it in the changeset context, implementing `atomic/3` as well as `change/3` so it does not force actions off the atomic path. A transaction cannot span two tenants -- separate files on separate connections, and SQLite cannot commit atomically across databases in WAL mode even with `ATTACH`. A statement for another tenant inside an open transaction is refused rather than committing on its own and surviving the rollback around it. ## global? `global? true` is honoured: such a resource is not required to carry a tenant. Ecto binds per repo *module*, though, so a global resource sharing a repo with tenanted ones reads from whichever tenant the process last bound. The tests say so rather than leaving it to be discovered. ## Documentation Deliberately none in this PR. The guide is being written once the whole feature has landed, rather than in pieces that contradict each other.
C-Sinclair
force-pushed
the
feat/multitenancy-engine
branch
from
August 24, 2026 14:43
b224ec9 to
41cca2e
Compare
…t of the box Stacked on the tenant binder PR, which ships the seam and nothing behind it. This supplies the runtime, so that `strategy :context` needs no binder of your own -- what @zachdaniel asked for on that PR: context multitenancy that manages its own storage rather than leaving it to the reader. children = [ MyApp.Repo, {AshSqlite.MultiTenancy, repo: MyApp.Repo, dir: "priv/tenants", migrations_path: "priv/repo/tenant_migrations"} ] `AshSqlite.MultiTenancy.Binder` becomes the default for a `strategy :context` resource, so the compile-time requirement to name one goes with it. Naming your own still overrides it, which is what keeps Turso, Litestream or a replica-aware router implementable outside this library. Named after `AshPostgres.MultiTenancy` rather than inventing a word for it. - `AshSqlite.MultiTenancy.Registry` -- one connection per tenant, and only one. - `AshSqlite.MultiTenancy.Connection` -- opens the file, migrates it, and refuses to serve until both have happened. - `AshSqlite.MultiTenancy.Manager` -- activation, an LRU bound on residency that will exceed itself rather than close a tenant mid-statement, quarantine for tenants that cannot open, and seal/unseal for draining a node. - `AshSqlite.MultiTenancy.Database` -- the tenant-to-filename mapping, escaped rather than sanitised so that it is injective: sanitising maps `a:b` and `a_b` onto one file, which puts two tenants in one database. - `AshSqlite.MultiTenancy.Migrations` -- compiles a migration directory once per boot rather than once per tenant. `all_tenants/1` answers what AshPostgres asks the application for through `Repo.all_tenants/0`; here it is derived, because a tenant is a file. Residents are unioned in, since SQLite creates the file on the first write. `rename/3` moves a tenant's database, sidecars included. Without it a renamed tenant keeps none of its data: the new name addresses a file that does not exist, and the next request quietly creates an empty one. The sidecars are not belt-and-braces -- a write is still in the WAL when the rename happens. Deliberately none, as with the PR below it. The guide covering both is a separate piece of work.
C-Sinclair
force-pushed
the
feat/multitenancy-engine
branch
from
August 24, 2026 15:54
41cca2e to
76866fc
Compare
Four defects in the tenant runtime, each found by probing rather than reading,
and each with the test that fails without the fix.
## A tenant was not held closed across its own rename or delete
`Binds.forget/2` deleted the closing mark along with the bind count, so the
`held_open_by_caller?` guard in `close/3` was defeated by the line above it.
`rename/3` and `delete/2` both document that they hold a tenant closed across
the file operation; neither did, from `close/3`'s return until their own `after`.
Measured, for rename: a write addressed to `acme` in that window commits into
`globex`'s database. For delete it is worse -- an activation there leaves the
tenant *resident* across the unlink, serving reads and accepting writes against
an inode with no directory entry, all of it discarded when the connection
closes. `all_tenants/1` unions residents, so `migrate_all/3` would then migrate
a tenant that no longer exists.
`forget/2` now drops only the binds and the last-used mark. The closing mark is
lifecycle state owned by whoever took it, and `close/3` already clears its own.
`delete/2` now takes a mark and holds it across the close and the unlink, as
`rename/3` does -- it never took one at all, so the `forget/2` change alone left
it broken.
## `close/3` closed a tenant with a statement in flight, silently
`await_quiescence/3` counted down and then closed regardless. `rename/3` calls
`close/3` without `force`, so a busy tenant had its file moved out from under a
live statement, which keeps the old inode and commits into the destination.
`close/3` now returns `{:error, :busy}`; `force: true` closes regardless, which
is what eviction and `delete/2` want once they know nothing is bound. `rename/3`
propagates it. `migrate_all/3` treats it as success and leaves the tenant
resident -- `close_after?` frees residency, it is not part of migrating -- and
forwards `:grace_ms`.
`grace_ms` is a deadline rather than a loop count. `Process.sleep(1)` sleeps at
least a millisecond, so it meant between one and several times what it said.
## A bind could be lost to an eviction that had already chosen its candidate
`Binds.bound/2` checked the closing mark and then incremented, so a bind could
slip between an eviction's choice of candidate and its close. It now increments
first and backs out if it lost: the two are separate ETS objects and no single
operation covers both, but publishing the increment before the check means a
closer can never read zero for a bind that goes on to proceed, which is the
direction that loses data.
That only holds because eviction cooperates, so `evict_if_needed/1` marks its
candidate closing *before* re-reading the count, and backs out if a bind got in.
Neither half is sufficient alone.
## A split read/mutate repo silently unbound every read
`put_dynamic_repo/1` binds one repo *module*. The default binder binds the
mutate repo, while reads resolve `repo(resource, :read)` -- so a `repo` function
returning different modules issued its reads unbound, against whatever database
that module was configured with. `VerifyRepo` returns early for any function
repo, so nothing checked it.
`VerifyTenantRepo` refuses the split, but only for a resource using the default
binder. A binder of its own is told whether each statement is a `:read` or a
`:write`, so routing reads to a replica is something only it can do correctly,
and forbidding that outright would be wrong.
## Also
`connection_for/2` on a repo with no fleet running raised `unknown registry:
MyApp.Repo.TenantRegistry`, naming an implementation detail instead of the
omission. It now names what is missing and shows the child spec, while
reraising untouched anything from a fleet that is actually running.
Tests promoted from the probes that found these: concurrent activation of one
cold tenant sharing a connection, statements surviving eviction churn under
contention, isolation holding across that churn, a tenant reused after a delete,
and the Ash-level create/read/update/destroy and bulk-create paths, which had no
coverage through Ash at all.
…n the caller's binding
`global?` means one copy of the rows. For a schema-based data layer that falls
out for free -- the global table sits in a schema the connection a tenanted
statement already holds can reach. One SQLite database per tenant has no such
connection, so it has to be chosen, and nothing chose it.
Two halves were wrong, and both are fixed here:
* A tenantless statement ran on whatever the calling process had bound. A
`global?` resource sharing a repo module with tenanted ones therefore read
whichever tenant was bound last, and which one depended on what that process
had done before.
* A statement *with* a tenant was bound to it, so a write with `tenant: "acme"`
landed in acme's file and every tenant accumulated its own copy of a table
that is supposed to have exactly one.
A `global?` resource now binds its repo module's own named instance, explicitly,
and ignores the tenant. Which database holds the global rows follows from `repo`:
sharing the tenanted module puts them in that module's configured database,
naming another module puts them there. Neither depends on the caller.
## Why not a tenant binder callback
The obvious shape was an optional `bind_global/2` on `AshSqlite.TenantBinder`,
letting a binder pick the connection for a tenantless statement. It is the wrong
seam. A binder picks a connection *instance* within one repo module; shared rows
are not another instance of a tenant's database but another database, which Ash
and Ecto already address as a repo module and resolve through
`AshSqlite.DataLayer.Info.repo/2`. Routing by binding would have put connection
selection in two places with no rule for which wins, and would still have needed
the answer `repo` already gives.
The binder contract is unchanged and no binder needs updating.
## Why the shared repo is checked at runtime
A `global?` resource needs a repo module started under its own name *and* holding a
`database:`. A repo module serving only tenants needs neither -- it is reached
through `Ecto.Repo.put_dynamic_repo/1` -- so adding `global? true` to a resource on
one is an easy mistake, and both halves are checked because neither implies the
other:
* Unstarted, the statement fails with Ecto's own "could not lookup Ecto repo",
which names the repo but not the reason a `global?` resource wanted it.
* Started with no `database:` -- which Ecto allows -- the statement waits out the
pool timeout and then reports that requests are arriving faster than they can be
served. Measured at roughly six seconds, and nothing in it points at the
missing configuration.
Neither check can be a transformer. A repo's `database:` is very often set in
`config/runtime.exs`, which is the recommended shape for a release, and a
transformer runs long before that file is evaluated -- so it would reject exactly
the configuration it should accept. They run instead when a global statement first
needs the connection, where the answer is knowable.
Nothing changes for a repo module with no `global?` resource on it.
`AshSqlite.ManagedTenantRepo` is configured with nothing whatsoever -- no
`database:`, no pool, no name -- and stays that way, with a test that says so.
## Tests
`AshSqlite.TenantRepo` gains a database and is started under its own name --
which is what a shared database is here -- while deliberately continuing to serve
the tenanted resources, so the tests are pointed at the footgun this fixes: a
global resource sharing a repo module with tenanted ones, still reading one copy.
`AshSqlite.Test.UnstartedGlobalPost` covers both ways to have no shared database,
on a repo module that has neither a name nor a database of its own.
The three tests that pinned the old behaviour are replaced rather than adjusted.
They asserted that a tenantless read followed the process binding and that a
tenanted write landed in the tenant's file; both are now wrong on purpose.
C-Sinclair
marked this pull request as ready for review
August 27, 2026 00:32
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Important
Stacked on #224 — please review and merge that first. GitHub cannot express a stacked PR across forks, so the diff below includes #224's commits (and #223's) as well.
To see only this PR's changes, either open this PR's own diff, or on the Files changed tab set the "Changes from all commits" dropdown to the single commit
improvement: manage tenant databases.#224 ships the tenant-binder seam and nothing behind it. This is the runtime behind it, so that
strategy :contextneeds no binder of your own — what you asked for, @zachdaniel: context multitenancy that manages its own storage rather than leaving it to the reader.That is the whole setup. A resource needs only
strategy :context;AshSqlite.MultiTenancy.Binderbecomes its default binder, so the compile-time requirement #224 imposes goes away. Naming your own still overrides it, which is what keeps Turso, Litestream or a replica-aware router implementable outside this library.What's in the box? 📦
Registry— one connection per tenant, and only one. A second is refused rather than racing.Connection— opens the file, migrates it, and refuses to serve until both have happened. A migration that raises stops the connection instead of serving an unmigrated database.Manager— activation, an LRU bound on residency that will exceed itself rather than close a tenant with a statement in flight, quarantine for tenants that cannot open, and seal/unseal for draining a node.Database— the tenant-to-filename mapping. Escaped rather than sanitised, so it is injective: sanitising mapsa:banda_bonto one file, which quietly puts two tenants in one database. Also why../../etc/xstays one file inside the directory.Migrations— compiles a migration directory once per boot rather than once per tenant.Things to note
all_tenants/1answers what AshPostgres asks the application for throughRepo.all_tenants/0, which raises until you define it. Here it can be derived, because a tenant is a file. Since SQLite only creates the actual file on the first write, we need to include the list of active residents as well. Otherwisemigrate_all/3could skip a tenant that had just been activated.rename/3moves a tenant's database, sidecars included. Without it a renamed tenant keeps none of its data: the new name addresses a file that does not exist, and the next request quietly creates an empty one. The sidecars are important — a write is still in the WAL at rename time. 4 tests added which fail if WAL files are left behind.Not in this PR
strategy :contextmultitenancy via a tenant binder #224. The guide covering the whole feature is a separate piece of work.mix ash_sqlite.generate_migrationshas no tenant awareness, so the directory passed as:migrations_pathis yours to maintain. AshPostgres routes tenanted resources topriv/repo/tenant_migrations; matching that is not attempted here.--tenants/--only-tenants/--except-tenants.MultiTenancy.migrate_all/3is the runtime equivalent.mix ash_postgres.rollback --tenants. There is no counterpart, by choice — forward-only for now.manage_tenant do ... end. Creation here is implicit — activation makes the file — andrename/3anddelete/2exist as primitives, but nothing calls them from a resource yet.