search

elixir-ecto / db_connection

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

Cleanly shutdown ownership and sojourn pools (#94) #95

Closed josevalim closed 7 years ago

josevalim commented 7 years ago

@fishcakez I am not quite sure how to solve this for sbroker. It seems it has an init callback but it doesn't have a terminate one?

fishcakez commented 7 years ago

@josevalim the reason for that it is only used as configuration, like a supervisor, and the callback is called on appups for reconfiguration.

We would have to have a process similar to the file server in otp that lives in the db_connection supervision tree and handles the start/stop of the pool for sbroker. TBH I would take any opportunity to deprecate the sojourn pool as it adds complications, such as this. sbroker does not fit well with the traditional pooling supervision layout because it is not designed to be used this way.

josevalim commented 7 years ago

@fishcakez ok, so for now let's merge this if you agree and we can revisit what to do regarding sojourn in an eventual 2.0.

fishcakez commented 7 years ago

I think we should solve it for both pools as we are still supporting them. I think this will require having a GenServer that links to ownership manager or the sojourn broker. In the case of a crash of the pool (monitored) it should send an exit signal (that is abnormal, perhaps kill to ensure a trapping exit broker goes down) to the manager/broker. In a case of a crash of the manager it should terminate the pool. What do you think?

josevalim commented 7 years ago

@fishcakez basically we take the watcher out of the pool supervisor, put them in their own supervisor, and make sure it terminates and send the proper exit signals? That sounds good to me.

fishcakez commented 7 years ago

@josevalim yeah

josevalim commented 7 years ago

@fishcakez I have implemented a new PR that uses a central DBConnection.Watcher as we discussed.

fishcakez commented 7 years ago

I will fixup this branch

josevalim commented 7 years ago

:heart: :green_heart: :blue_heart: :yellow_heart: :purple_heart: