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

MID-02 - Implement transactional middleware #1

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

vereis
Copy link
Owner

@vereis vereis commented Nov 13, 2024

This PR is a potential implementation of transactional middleware.

Today, all EctoMiddleware behaviours are assumed to not be transactional, which might be fine for certain use-cases, but not for others.

For example, if one wanted to create an event log by hooking into all CRUD operations today, there is a very real risk that it would not be a sound implementation because writing to the events log would not be atomic with the CRUD operation.

This PR introduces a new optional callback to the EctoMiddleware behaviour: transactional?/0, which, if implemented and set to true, will:

  1. Wrap the Ecto.Repo callback (such as get_by/2) in a transaction
  2. Execute the transactional middleware either before or after the original get_by/2 implementation

If no configured EctoMiddleware are marked as transactional? then we avoid starting a transaction as this may be quite expensive.

In reality, it's hard to foresee a usecase where an EctoMiddleware implementation would always want to run inside a transaction... this would certainly tank performance.

Due to this, EctoMiddleware now exports a set_transactional/2 function which can be used to toggle middleware to be transactional on a middleware by middleware, process by process basis.

Note that if you have a mix of middleware configured which are transactional and non-transactional, the execution order of the middleware is as follows:

  1. Run all the before_middleware which are not transactional
  2. Maybe start a transaction, and run all the before_transactional_middleware if these exist
  3. Run the original Ecto.Repo callback
  4. Maybe run all the after_transactional_middleware if these exist
  5. If the transaction was started, commit it
  6. Run all the after_middleware which are not transactional

Before merging this PR in, we should really also implement a test suite.

Additional changes to EctoHooks which is the driver for this change will be implemented in tandem with this change.

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

Successfully merging this pull request may close these issues.

1 participant