elixir-ecto / db_connection

Database connection behaviour
http://hexdocs.pm/db_connection/DBConnection.html
306 stars 113 forks source link

Add owner feature to connection #1

Closed josevalim closed 8 years ago

josevalim commented 8 years ago

See: https://github.com/robconery/moebius/issues/37

If the owner dies, the connection process should exit.

fishcakez commented 8 years ago

Should this start a pool that starts a connection for each DBConnection.run/3 or is a pool of 1 started that monitors the owner?

josevalim commented 8 years ago

@fishcakez I don't think we need a pool. I would like the connection to simply monitor the process given as owner. If the owner crashes, the connection should shut down. We could also use link to ensure immediate shut downs.

fishcakez commented 8 years ago

Sorry I meant should a connection be started with start_link each time?

josevalim commented 8 years ago

For moebius case in particular I was thinking they would put it behind a simple_one_for_one supervisor that calls start_link with the owner. They would guarantee the shut down, so no pool. Or am I misunderstanding?

fishcakez commented 8 years ago

Do we start the simple supervisor and spawn a work for the duration of a checkout like so:

## start a simple one for one in any applications tree
{:ok, pid} = DBConnection.start_link(adapter, [pool_mod: DBConnection.Ownership])
## starts a connection under the supervisor that exits after run/3 finishes or the caller exits.
DBConnection.run(pid, fn(conn) -> ... end)

Or a connection is started explicitly:

## Starts and links to a connection under a simple supervisor in DBConnection tree.
{:ok, pid} = DBConnection.start_link(adapter, [pool_mod: DBConnection.Ownership])
## This time there are multiple checkouts
DBConnection.run(pid, fn(conn) -> ... end)
DBConnection.run(pid, fn(conn) -> ... end)

The later case allows starting inside a GenServer's init/1 and reusing the connection throughout, whereas the former does not.

The former case allows connection configuration to be given both at start_link and run, whereas all configuration is given at start_link in the later so all processes using a connection would need to know all the configuration.

I think the former is what moebius would like but the later what Ecto would like 2.0?

fishcakez commented 8 years ago

I know I said I thought proxy modules were great for ownership but I am not sure they are for this. I think it might be easier for ownership to require its ownership callbacks (as you original planned).

Trying to use proxy modules for real I am quite disappointed with my implementation.

josevalim commented 8 years ago

Trying to use proxy modules for real I am quite disappointed with my implementation.

That's good though, it means we have room for improvements. Don't worry about the proxy stuff for now. My plan is to work on ownership soon and we can re-access the proxy stuff together by then.