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

Adapter config for default_transaction_mode: IMMEDIATE #153

Closed
egze opened this issue Oct 26, 2024 · 6 comments
Closed

Adapter config for default_transaction_mode: IMMEDIATE #153

egze opened this issue Oct 26, 2024 · 6 comments

Comments

@egze
Copy link
Contributor

egze commented Oct 26, 2024

I am trying to use sqlite in a project and have noticed that there is no option to set a IMMEDIATE transaction mode globally.

This has been brought up before here: #151 and the conclusion was to have the responsibility on the caller.

I want to raise the question again to have a configuration on the adapter level and maybe even make it default.

Here are 2 convincing sources:

  1. https://fractaledmind.github.io/2024/04/15/sqlite-on-rails-the-how-and-why-of-optimal-performance/
    The author did extensive testing with Rails and concluded that it is not viable to have DEFERRED in a typical Rails application. And to me a typical Rails application has the same usage pattern as a Phoenix application.

In a context where you only have one connection or you have a large amount of transactions that only do read operations, this is great for performance, because it means that SQLite doesn’t have to acquire a lock on the database for every transaction, only for transactions that actually write to the database. The problem is that this is not the context Rails apps are in. In a production Rails application, not only will you have multiple connections to the database from multiple threads, Rails will only wrap database queries that write to the database in a transaction. And, when we write our own explicit transactions, it is essentially a guarantee that we will include a write operation. So, in a production Rails application, SQLite will be working with multiple connections and every transaction will include a write operation. This is the opposite of the context that SQLite’s default deferred transaction mode is optimized for.

  1. https://highperformancesqlite.com/watch/transaction-modes
    Aaron Francis is a well known database expert and his sqlite course is amazing. Here are some quotes:

So on the application side, whenever you connect to the database, you're going to want to set your transaction mode to immediate.

You'll need to look up for your specific framework how to connect to the database and how to make sure that all of the rights are wrapped in a begin immediate transaction. Most web frameworks wrap the rights in a transaction regardless, and so you're looking for the thing where you can set, alright, we want that to be an immediate transaction. I think the rails adapter for SQLite, I think that does that by default. I know in PHP, you can set a p d o attribute to set it to immediate. So you'll have to do a little bit of digging.

To summarise: IMMEDIATE transaction mode makes the most sense for a web application. Passing manually mode: :immediate is not a good experience.

Main question: Are the maintainers open to a config on adapter level? In Rails the config is called default_transaction_mode: IMMEDIATE, we could do the same.

I am happy to do the work, with some pointers where to start best.

@warmwaffles
Copy link
Member

It doesn't bother me if someone adds it. Right now I've settled on using:

defmodule MyApp.Repo do
  use Ecto.Repo, adapter: Ecto.Adapters.SQLite3, otp_app: :my_app
  
  def immediate_transaction(fun_or_multi) do
    transaction(fun_or_multi, mode: :immediate)
  end
end

I have a few cases in my personal project where I want deferred and opt into immediate mode for transactions. This has to do with multiple processes reading from the database at the same time. In my use case, I wanted as low as possible blocks to reads as I can because this is a heavy read application with low writes.

If you want to add the option, you are more than welcome to try it out. Although you will need to do that change in exqlite because that is where the DBConnection interface is defined.

https://github.com/elixir-sqlite/exqlite/blob/81cf923b979eaee1ae748a516b39452046e64592/lib/exqlite/connection.ex#L252-L281

Keep in mind, we are going through some reworking of the underlying adapter sqlite implementation and the ecto adapter to better consolidate responsibilities.

You'll need to add to the %Connection{} struct a new field :default_transaction_mode and default it to :deferred since that was the original implementation.

def handle_begin(options, %{transaction_status: transaction_status, state.default_transaction_mode: state.default_transaction_mode} = state) do
  mode = Keyword.get(options, :mode) || default_transaction_mode || :deferred
  # ...
end

Then you'll probably need to shim that default mode into do_connect/2

  defp do_connect(database, options) do
    # ... with block ....
    state = %__MODULE__{
      default_transaction_mode: Keyword.get(options, :default_transaction_mode, :deferred)
    }
    # ...
  end

You'll then need to update the connection_opt() type definition to include the new transaction mode option and finally add it to the doc block.

@egze
Copy link
Contributor Author

egze commented Oct 28, 2024

Thanks for the tips. I'll pick it up in a week.

@warmwaffles
Copy link
Member

Personally though, I still think having the helper function defined on the Repo is far more telling to the reader that this transaction is deferred vs immediate.

I have this in my application's Repo definition

  def immediate_transaction(fun_or_multi) do
    transaction(fun_or_multi, mode: :immediate)
  end

  def deferred_transaction(fun_or_multi) do
    transaction(fun_or_multi, mode: :deferred)
  end

And use it like multi |> Repo.deferred_transaction() or multi |> Repo.immediate_transaction()

BUT, with the changes I suggested above for the default transaction mode it won't break my application and is a reasonable ask.

@egze
Copy link
Contributor Author

egze commented Nov 3, 2024

@warmwaffles I create a PR for exqlite. When it's merged, I'll add one for the current repo with some more documentation.

@egze
Copy link
Contributor Author

egze commented Nov 4, 2024

@warmwaffles And the follow-up PR with docs: #155

@warmwaffles
Copy link
Member

Awesome. Thank you for knocking this out.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants