search

elixir-ecto / db_connection

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

Add shared ownership mode #21

Closed josevalim closed 8 years ago

fishcakez commented 8 years ago

Do we need shared and automatic? We could run tests with shared.

josevalim commented 8 years ago

Do we need shared and automatic? We could run tests with shared.

We need automatic because it allows multiple processes to run with different connections.

fishcakez commented 8 years ago

Could we get a test to go with that last commit? I think something like changing the mode explicitly from shared to automatic we correctly handle the exit of the shared pid and can get an automatic checkout afterwards. That is a complicated case we could easily regress on. Otherwise I think we are good to ship this.

josevalim commented 8 years ago

I tried to but failed. Let me give it another ago. :D

josevalim commented 8 years ago

The reason I am getting stuck in the test is because we are waiting for a process to exit and there is a race condition as the manager may not have processed the DOWN message in time. So we were supposed to be able to checkout but we cannot because the manager still didn't receive the DOWN message. :(

fishcakez commented 8 years ago

But the mode has been changed to automatic at that point (before shared exit), so we should be able to automatically start an owner that blocks until the connection owned by the shared process checks in?

fishcakez commented 8 years ago

Oh wait I see. Ideally we want to checkout from the manager after the DOWN is handled.

josevalim commented 8 years ago

@fishcakez pushed a test now. The tracing tip was great!

josevalim commented 8 years ago

@fishcakez let me know if you approve this PR and I will cut a new release :D

fishcakez commented 8 years ago

:shipit: