Browse Source

Remove and do not auto-add TCP option nodelay if not supported (#184)

Jan Uhlig 3 years ago
parent
commit
076bdef2c9
2 changed files with 48 additions and 7 deletions
  1. 9 7
      src/mysql_conn.erl
  2. 39 0
      test/mysql_tests.erl

+ 9 - 7
src/mysql_conn.erl

@@ -159,7 +159,7 @@ connect(#state{connect_timeout = ConnectTimeout} = State) ->
 
 connect_socket(#state{tcp_opts = TcpOpts, host = Host, port = Port} = State) ->
     %% Connect socket
-    SockOpts = sanitize_tcp_opts(TcpOpts),
+    SockOpts = sanitize_tcp_opts(Host, TcpOpts),
     {ok, Socket} = gen_tcp:connect(Host, Port, SockOpts),
 
     %% If buffer wasn't specifically defined make it at least as
@@ -175,12 +175,13 @@ connect_socket(#state{tcp_opts = TcpOpts, host = Host, port = Port} = State) ->
 
     {ok, State#state{socket = Socket}}.
 
-sanitize_tcp_opts([{inet_backend, _} = InetBackend | TcpOpts0]) ->
+sanitize_tcp_opts(Host, [{inet_backend, _} = InetBackend | TcpOpts0]) ->
     %% This option is be used to turn on the experimental socket backend for
     %% gen_tcp/inet (OTP/23). If given, it must remain the first option in the
     %% list.
-    [InetBackend | sanitize_tcp_opts(TcpOpts0)];
-sanitize_tcp_opts(TcpOpts0) ->
+    [InetBackend | sanitize_tcp_opts(Host, TcpOpts0)];
+sanitize_tcp_opts(Host, TcpOpts0) ->
+    NodelaySupported = not is_tuple(Host) orelse element(1, Host) =/= local,
     TcpOpts1 = lists:filter(
         fun
             ({mode, _}) -> false;
@@ -188,13 +189,14 @@ sanitize_tcp_opts(TcpOpts0) ->
             (list) -> false;
             ({packet, _}) -> false;
             ({active, _}) -> false;
+            ({nodelay, _}) -> NodelaySupported;
             (_) -> true
         end,
         TcpOpts0
     ),
-    TcpOpts2 = case lists:keymember(nodelay, 1, TcpOpts1) of
-        true -> TcpOpts1;
-        false -> [{nodelay, true} | TcpOpts1]
+    TcpOpts2 = case NodelaySupported andalso not lists:keymember(nodelay, 1, TcpOpts1) of
+        true -> [{nodelay, true} | TcpOpts1];
+        false -> TcpOpts1
     end,
     [binary, {packet, raw}, {active, false} | TcpOpts2].
 

+ 39 - 0
test/mysql_tests.erl

@@ -261,6 +261,45 @@ unix_socket_test() ->
                                   "release could not be determined.~n")
     end.
 
+socket_backend_test() ->
+    try
+        list_to_integer(erlang:system_info(otp_release))
+    of
+        %% Supported in OTP >= 23
+        OtpRelease when OtpRelease >= 23 ->
+            case mysql:start_link([{user, ?user},
+                                   {password, ?password},
+                                   {tcp_options, [{inet_backend, socket}]}])
+            of
+                {ok, Pid1} ->
+                    {ok, [<<"@@socket">>], [[SockFile]]} =
+                                 mysql:query(Pid1, <<"SELECT @@socket">>),
+                    mysql:stop(Pid1),
+                    case mysql:start_link([{host, {local, SockFile}},
+                                           {user, ?user}, {password, ?password},
+                                           {tcp_options, [{inet_backend, socket}]}]) of
+                        {ok, Pid2} ->
+                            ?assertEqual({ok, [<<"1">>], [[1]]},
+                                         mysql:query(Pid2, <<"SELECT 1">>)),
+                            mysql:stop(Pid2);
+                        {error, eafnotsupported} ->
+                            error_logger:info_msg("Skipping socket backend test. "
+                                                  "Not supported on this OS.~n")
+                    end;
+                {error, enotsup} ->
+                    error_logger:info_msg("Skipping socket backend test. "
+                                          "Not supported on this OS.~n")
+            end;
+        OtpRelease ->
+            error_logger:info_msg("Skipping socket backend test. Current OTP "
+                                  "release is ~B. Required release is >= 23.~n",
+                                  [OtpRelease])
+    catch
+        error:badarg ->
+            error_logger:info_msg("Skipping socket backend tests. Current OTP "
+                                  "release could not be determined.~n")
+    end.
+
 connect_queries_failure_test() ->
     process_flag(trap_exit, true),
     {ok, Ret, Logged} = error_logger_acc:capture(