Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discussion of how to deal with concurrent write transactions. #91

Open
jimsynz opened this issue Oct 7, 2024 · 1 comment
Open

Discussion of how to deal with concurrent write transactions. #91

jimsynz opened this issue Oct 7, 2024 · 1 comment
Labels
enhancement New feature or request

Comments

@jimsynz
Copy link
Contributor

jimsynz commented Oct 7, 2024

So there's a small issue with Sqlite where only one transaction can obtain a write lock at a time, and any attempt to write (not even commit) while another transaction has a write lock causes an error which leaves no option other than rolling back the failed transaction.

Obviously this is not how Ash expects a transaction supporting data layer to behave, which is why transactions are explicitly disabled in ash_sqlite. Unfortunately there's not really any indication that this is the default as there's nothing in the docs that indicate that this is the case - in fact there's a couple of places that talk about using transactions.

There is reason to hope that Sqlite will get at least less surprising transaction behaviour via the BEGIN CONCURRENT proposal, however it currently lives outside of the main tree and is not shipped by default.

Proposals:

  1. Doc changes that very prominently explain ash_sqlite's inability to run actions in a transaction.
  2. A guide to using a reactor to replace action hooks for transaction-like behaviour.
  3. A verifier (in core) that explicitly warns when any action does not have transaction? false when the data layer does not support transacting.
  4. A runtime check (in core) that raises an error when before or after action hooks are used with a data layer that does not support transacting.
  5. A verifier (in core) that verifies that the resource's data layer supports transacting when using the before_action and after_action builtin changes.
  6. Engage with ecto_sqlite3 and exqlite package maintainers to try and find a better solution.

Relevant links:

@jimsynz jimsynz added the enhancement New feature or request label Oct 7, 2024
@jimsynz
Copy link
Contributor Author

jimsynz commented Oct 9, 2024

Okay, after having a shower thought and discussing it with @zachdaniel here's a new proposal:

  1. Add write_transactions? to the sqlite DSL section so that folks can explicitly opt in to transaction support.
  2. Add AshSqlite.select_repo/2 which implements the expected behaviour for the sqlite.repo DSL callback - this currently doesn't exist and will have to be copied from postgres.repo.

The idea being that folks can create a separate repo just for writes, and configure the pool size to 1 which effectively forces all write transactions to be serialised just how sqlite wants them. We can even add an igniter code patcher which makes the required changes to existing repos in a codebase.

I still think proposals 3..6 from above are important and 1 should be modified to provide a guide to managing transactions with SQLite including the above solution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Status: Someday
Development

No branches or pull requests

1 participant