Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

connection broken does not trigger a reconnection #37

Open
wentianxiaokai opened this issue Feb 28, 2018 · 7 comments
Open

connection broken does not trigger a reconnection #37

wentianxiaokai opened this issue Feb 28, 2018 · 7 comments

Comments

@wentianxiaokai
Copy link

wentianxiaokai commented Feb 28, 2018

     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
Copy link
Author

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
Copy link

dhull commented Mar 15, 2018

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
Copy link
Author

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
Copy link

dhull commented Mar 15, 2018

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
Copy link
Author

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
Copy link

dhull commented Mar 15, 2018

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
Copy link
Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants