search

samsondav / rihanna

Rihanna is a high performance postgres-backed job queue for Elixir
MIT License
439 stars 49 forks source link

Support Ecto Repo #38

Closed lpil closed 5 years ago

lpil commented 5 years ago

Hello!

Ecto has a nice sandbox that allows us to run database hitting tests concurrently when using Ecto's connection pool.

It'd be nice to be able to use this with Rihanna. Currently the library is hard-coded to use a dedicated Postgrex connection, however if the user was able to pass a connection the user could pass an Ecto repo and thus make use of the sandbox.

This would also remove the need for the Rihanna.Job.Postgrex in many cases, halving the number of database connections that Rihanna uses. Configuration could be added to prevent this process from being started if desired.

In addition it would also enable the user to enqueue more jobs and perform business login with the same SQL transaction as the connection will be shared across both automatically with Ecto, enabling a little more safety.

Here's some prior art on adding Ecto integration to a SQL executing lib without coupling tightly https://github.com/lpil/yesql/commit/e692828c572781d75f3575a6910eb2848cc6d1ce

I will implement this feature if given the thumbs up.

Cheers, Louis

lpil commented 5 years ago

It could be specified via mix config, as with the other configuration in Rihanna.

By default it continues to behave as it does today, however this config could be specified like so:

# A named postgrex connection is used
config :rihanna,
  enqueue_postgres_connection: {Postgrex, :my_postgrex_connection}
# An Ecto connection pool is used
config :rihanna,
  enqueue_postgres_connection: {Ecto, My.Repo}

In either of the above cases the Rihanna.Job.Postgrex process is not started.

samsondav commented 5 years ago

Interesting idea. From what I understand, this is only for the process that enqueues jobs, is that correct?

lpil commented 5 years ago

What do you mean? :)

samsondav commented 5 years ago

I mean, it's for Rihanna.enqueue - not the dispatcher?

lpil commented 5 years ago

Yes, it's only for enqueuing.

Ecto doesn't offer a way to always request the same connection from the pool so the dispatcher so the advisory locks wouldn't work. I believe it is possible to check out a connection long term with Ecto, but this is effectively the same as having a dedicated Postgrex connection so I'm not sure I see an advantage.

samsondav commented 5 years ago

This seems like a good idea. Feel free to implement :)

lpil commented 5 years ago

Do you like the config design above?

samsondav commented 5 years ago

Yeah it looks ok.

lpil commented 5 years ago

It could be passed at runtime for sure.

I suggested that API because Rihanna doesn't currently accept configuration beyond the Postgrex config at runtime so I thought that would be more in keeping with the current API.