dizzyd / erlang-mysql-driver

Erlang MySQL Driver (from code.google.com)
Other
175 stars 105 forks source link

connection broken does not trigger a reconnection #37

Open wentianxiaokai opened 6 years ago

wentianxiaokai commented 6 years ago

     I had a trouble about firewall when I use this library.The phenomenon is: mysql client A which uses erlang-mysql-driver library connect to mysql server B,and can access to B normal.But after two hours the socket is prevented by firewall,the Keep-Alive msg from B to A is prevented by firewall,the socket is broken,but A don't know!When A send a mysql query msg to B ,then would get a msg said "Failed sending data on socket",Nor would reconnect to B.      By reading the source code,we know A will reconnect to B only when A receive a "tcp_closed" msg,ie,a tcp_fin msg.So A will not receive "tcp_closed"/tcp_fin msg when firewall work or the socket is broken in some way,nor will reconnect to B.And A will not remove the connection.All of operations just get a return value "Failed sending data on socket".      So my solution is that add some codes in mysql_conn.erl:470 of do_query(Sock, RecvPid, LogFun, Query, Version)
gen_tcp:close(Sock), RecvPid ! {tcp_closed, Sock},      That is,when do_send() error,we close this socket,and product a tcp_closed msg,send to the RecvPid.This can notice RecvPid to trigger the reconnection.      Finally,the author add code for mysql heartbeat and reconnect automatically is the best way!

wentianxiaokai commented 6 years ago

Last solution is a wrong answer.The right answer is to add "erlang:monitor(process, RecvPid)," in mysql_recv.erl:73,after spawn_link()。

dhull commented 6 years ago

I just ran into this problem myself, however I think that suggested solution is wrong. I have not yet tested this, but I believe that at mysql_conn.erl:470 the {error, Msg} should be changed to exit(closed). The mysql_conn process will exit, and the mysql gen_server, which is monitoring it, will receive a DOWN message at mysql.erl:626 and remove the connection from the pool. I have looked at the errors that gen_tcp:send can return and it doesn't look to me like there's any good way to recover from any of them.

wentianxiaokai commented 6 years ago

I think that change mysql_conn.erl:470 '{error, Msg}' to 'exit(closed)' is one way when user notice it by gen_tcp send error.My solution of adding "erlang:monitor(process, RecvPid)," in mysql_recv.erl:73 after "spawn_link()" is another way,which can discover the disconnection of the socket immediately.

dhull commented 6 years ago

I tested my suggestion and found I had to use exit(self(), closed) instead of exit(closed). The problem is that an exit(closed) is caught by the catch in mysql_conn:do_queries/5. Using exit/2 bypasses the catch.

wentianxiaokai commented 6 years ago

Thanks.I didn't try your solution.I had try my solution of "erlang:monitor(process, RecvPid)," two weeks ago.And my application worked normal.

dhull commented 6 years ago

Looking further, there are definitely other problems in the mysql_conn closed-connection handling that your patch will fix that mine does not handle. My only suggestion is that I would move the monitor call to right under {ok, RecvPid, Sock} pattern match after the mysql_recv:start_link call in mysql_conn:init. The reason is that the DOWN message handling is in mysql_conn, so it make sense to me to set up the monitor in the same module.

wentianxiaokai commented 6 years ago

Fine!Your suggestion is equivalent to mine,since our solutions are pipelining in a process.