MB-48047:[BP] During config reload try connecting to kv on all

afamily and use the connection which works.

When we attempt to reload memcached config, we may have a situation
where the address family in ns_server has changed but not propogated to
memcached yet. We might fail to communicate with memcached over new address
family as memcached may not be listening on the new address family. This
bug is especially seen when we go from ipv4-only to ipv6-only and vice
versa.
Since we cannot know for sure which address family memcached is
listening on, due to various error paths that can lead to mismatch in
what ns_server perceive the memcached config is and what in fact is
applied in memcached, it is simplest to attempt connection using both
address family.

Backports change related to MB-43196.

Reviewed-on: http://review.couchbase.org/c/ns_server/+/142182

Change-Id: Id8fb97470bbd2bdb4573c8a01b16650cc652601b
Reviewed-on: http://review.couchbase.org/c/ns_server/+/164953
Tested-by: Abhijeeth Nuthan <[email protected]>
Reviewed-by: Timofey Barmin <[email protected]>
Well-Formed: Restriction Checker

Author: anuthan

src/memcached_config_mgr.erl

@@ -175,10 +175,20 @@ code_change(_OldVsn, State, _Extra) ->
     {ok, State}.
 
 apply_changed_memcached_config(DifferentConfig, State) ->
-    case ns_memcached:config_validate(DifferentConfig) of
+    %% We might have changed address family in ns_server and we don't want to
+    %% fail after trying to connect to the address family which might not exist
+    %% in memcached yet.
+    %% Since we cannot know for sure which address family memcached is
+    %% listening on, due to various error paths that can lead to mismatch in
+    %% what this module perceives the memcached config is and what in fact is
+    %% applied in memcached, it is simplest to attempt connection using both
+    %% address family.
+    AFamiliesToTry = [inet, inet6],
+
+    case ns_memcached:config_validate(DifferentConfig, AFamiliesToTry) of
         ok ->
             ?log_debug("New memcached config is hot-reloadable."),
-            hot_reload_config(DifferentConfig, State, 10, []),
+            hot_reload_config(DifferentConfig, AFamiliesToTry, State, 10, []),
             {noreply, State#state{memcached_config = DifferentConfig}};
         {memcached_error, einval, _} ->
             ?log_debug("Memcached config is not hot-reloadable"),
@@ -221,14 +231,14 @@ changed_keys(BlobBefore, BlobAfter) ->
             unknown
     end.
 
-hot_reload_config(NewMcdConfig, State, Tries, LastErr) when Tries < 1 ->
+hot_reload_config(NewMcdConfig, _, State, Tries, LastErr) when Tries < 1 ->
     ale:error(?USER_LOGGER,
               "Unable to apply memcached config update that was supposed to "
               "succeed. Error: ~p. Giving up. Restart memcached to apply that "
               "config change. Updated keys: ~p",
               [LastErr, changed_keys(State#state.memcached_config,
                                      NewMcdConfig)]);
-hot_reload_config(NewMcdConfig, State, Tries, _LastErr) ->
+hot_reload_config(NewMcdConfig, AFamiliesToTry, State, Tries, _LastErr) ->
     FilePath = get_memcached_config_path(),
     PrevFilePath = FilePath ++ ".prev",
 
@@ -243,7 +253,7 @@ hot_reload_config(NewMcdConfig, State, Tries, _LastErr) ->
     %% we'll be able to retry hot or cold config update
     ok = misc:atomic_write_file(FilePath, NewMcdConfig),
 
-    case ns_memcached:config_reload() of
+    case ns_memcached:config_reload(AFamiliesToTry) of
         ok ->
             delete_prev_config_file(),
             ale:info(?USER_LOGGER,
@@ -255,7 +265,8 @@ hot_reload_config(NewMcdConfig, State, Tries, _LastErr) ->
             ?log_error("Failed to reload memcached config. "
                        "Will retry. Error: ~p", [Err]),
             timer:sleep(1000),
-            hot_reload_config(NewMcdConfig, State, Tries - 1, Err)
+            hot_reload_config(NewMcdConfig, AFamiliesToTry, State,
+                              Tries - 1, Err)
     end.
 
 get_memcached_config_path() ->

src/ns_memcached.erl

@@ -116,8 +116,8 @@
          get_vbucket_high_seqno/2,
          wait_for_seqno_persistence/3,
          get_keys/3,
-         config_validate/1,
-         config_reload/0,
+         config_validate/2,
+         config_reload/1,
          get_failover_log/2,
          get_failover_logs/2,
          get_collections_uid/1
@@ -1184,13 +1184,27 @@ do_connect(Options) ->
     Port = service_ports:get_port(memcached_dedicated_port, Config),
     User = ns_config:search_node_prop(Config, memcached, admin_user),
     Pass = ns_config:search_node_prop(Config, memcached, admin_pass),
-    {ok, Sock} = gen_tcp:connect(misc:localhost(), Port,
-                                 [misc:get_net_family(),
-                                  binary,
-                                  {packet, 0},
-                                  {active, false},
-                                  {recbuf, ?RECBUF},
-                                  {sndbuf, ?SNDBUF}]),
+    AFamilies = proplists:get_value(try_afamily, Options,
+                                    [misc:get_net_family()]),
+    HelloFeatures = proplists:delete(try_afamily, Options),
+    {ok, Sock} = lists:foldl(
+                   fun (_AFamily, {ok, Socket}) ->
+                           {ok, Socket};
+                       (AFamily, Acc) ->
+                           RV = gen_tcp:connect(misc:localhost(AFamily, []),
+                                                Port,
+                                                [AFamily,
+                                                 binary,
+                                                 {packet, 0},
+                                                 {active, false},
+                                                 {recbuf, ?RECBUF},
+                                                 {sndbuf, ?SNDBUF}]),
+                           case RV of
+                               {ok, S} -> {ok, S};
+                               _ -> [{AFamily, RV} | Acc]
+                           end
+                   end, [], AFamilies),
+
     try
         case mc_client_binary:auth(Sock, {<<"PLAIN">>,
                                           {list_to_binary(User),
@@ -1201,7 +1215,7 @@ do_connect(Options) ->
                            "provided password <ud>~s</ud>", [User, Pass]),
                 error({auth_failure, Err})
         end,
-        Features = mc_client_binary:hello_features(Options),
+        Features = mc_client_binary:hello_features(HelloFeatures),
         {ok, Negotiated} = mc_client_binary:hello(Sock, "regular", Features),
         Failed = Features -- Negotiated,
         Failed == [] orelse error({feature_negotiation_failed, Failed}),
@@ -1447,18 +1461,20 @@ do_get_keys(Bucket, NodeVBuckets, Params) ->
               end
       end, NodeVBuckets, ?GET_KEYS_OUTER_TIMEOUT).
 
--spec config_validate(binary()) -> ok | mc_error().
-config_validate(NewConfig) ->
+-spec config_validate(binary(), [inet | inet6]) -> ok | mc_error().
+config_validate(NewConfig, AFamilies) ->
     misc:executing_on_new_process(
       fun () ->
-              {ok, Sock} = connect([{retries, 1}]),
+              {ok, Sock} = connect([{retries, 1},
+                                    {try_afamily, AFamilies}]),
               mc_client_binary:config_validate(Sock, NewConfig)
       end).
 
-config_reload() ->
+config_reload(AFamilies) ->
     misc:executing_on_new_process(
       fun () ->
-              {ok, Sock} = connect([{retries, 1}]),
+              {ok, Sock} = connect([{retries, 1},
+                                    {try_afamily, AFamilies}]),
               mc_client_binary:config_reload(Sock)
       end).