MB-49271 [BP] Takeover via orchestrator using conn names < 200 chars

This is a backport of:

MB-47866: Takeover via orchestrator using conn names < 200 chars
Reviewed-on: http://review.couchbase.org/c/ns_server/+/159528

Change-Id: I616d3ef9bdb681ce683011e3d5313e51d5570f40
Reviewed-on: http://review.couchbase.org/c/ns_server/+/164898
Well-Formed: Restriction Checker
Tested-by: Steve Watanabe <[email protected]>
Tested-by: Build Bot <[email protected]>
Reviewed-by: Abhijeeth Nuthan <[email protected]>

Author: stevewatanabe

src/dcp_replicator.erl

@@ -34,6 +34,7 @@
          setup_replication/2, setup_replication/3,
          takeover/2, takeover/3,
          wait_for_data_move/3,
+         trim_common_prefix/2,
          get_docs_estimate/3,
          get_connections/1]).
 
@@ -235,13 +236,8 @@ get_connection_name(ConsumerNode, ProducerNode, Bucket) ->
         true ->
             CName;
         false ->
-            %% Find the longest common prefix for the two nodes and chop
-            %% it off (but not below a minimal length).
-            LCP = binary:longest_common_prefix(
-                    [atom_to_binary(ConsumerNode, latin1),
-                     atom_to_binary(ProducerNode, latin1)]),
-            CNode = maybe_cut_name(ConsumerNodeList, LCP),
-            PNode = maybe_cut_name(ProducerNodeList, LCP),
+            %% Trim off the common prefix to shorten the names.
+            {CNode, PNode} = trim_common_prefix(ConsumerNode, ProducerNode),
 
             Hash = binary_to_list(base64:encode(crypto:hash(sha, CName))),
             Bkt = string:slice(Bucket, 0, 60),
@@ -252,6 +248,16 @@ get_connection_name(ConsumerNode, ProducerNode, Bucket) ->
             CName2
     end.
 
+trim_common_prefix(Consumer, Producer) ->
+    %% Find the longest common prefix for the two nodes and chop
+    %% it off (but not below a minimal length).
+    LCP = binary:longest_common_prefix(
+            [atom_to_binary(Consumer, latin1),
+             atom_to_binary(Producer, latin1)]),
+    Consumer1 = maybe_cut_name(atom_to_list(Consumer), LCP),
+    Producer1 = maybe_cut_name(atom_to_list(Producer), LCP),
+    {Consumer1, Producer1}.
+
 %% Cut the specified number of bytes from the front of the name but don't
 %% shorten below a minimum length.
 maybe_cut_name(Name, MaxToChop) ->
@@ -380,5 +386,4 @@ get_connection_name_test() ->
                  "AShortNodeName:travel-sample-with-a-very-very-very-very-"
                  "long-bucket-name:A3aPD1Sik+5ZIz43M6NNTGn9XFw=", Conn56).
 
-
 -endif.

src/ns_single_vbucket_mover.erl

@@ -19,6 +19,9 @@
 -export([spawn_mover/5, mover/6]).
 
 -include("ns_common.hrl").
+-ifdef(TEST).
+-include_lib("eunit/include/eunit.hrl").
+-endif.
 
 spawn_mover(Bucket, VBucket, OldChain, NewChain, Quirks) ->
     Parent = self(),
@@ -416,18 +419,39 @@ dcp_takeover_via_orchestrator(Bucket, Parent,
       end).
 
 start_takeover_replicator(NewMaster, OldMaster, Bucket, VBucket) ->
-    ConnName = lists:concat(["replication:takeover:",
-                             binary_to_list(couch_uuids:random()), ":",
-                             atom_to_list(OldMaster), "->",
-                             atom_to_list(NewMaster), ":",
-                             Bucket, ":",
-                             integer_to_list(VBucket)]),
+    ConnName = get_takeover_connection_name(NewMaster, OldMaster, Bucket,
+                                            VBucket),
 
     RepFeatures = dcp_sup:get_replication_features(),
     {ok, Pid} = dcp_replicator:start_link(undefined, NewMaster, OldMaster,
                                           Bucket, ConnName, RepFeatures),
     Pid.
 
+get_takeover_connection_name(NewMaster, OldMaster, Bucket, VBucket) ->
+    ConnName0 = lists:concat(["replication:takeover:",
+                              binary_to_list(couch_uuids:random()), ":",
+                              atom_to_list(OldMaster), "->",
+                              atom_to_list(NewMaster), ":",
+                              Bucket, ":",
+                              integer_to_list(VBucket)]),
+
+    case length(ConnName0) =< ?MAX_DCP_CONNECTION_NAME of
+        true ->
+            ConnName0;
+        false ->
+            {OldM, NewM} = dcp_replicator:trim_common_prefix(
+                             OldMaster, NewMaster),
+            ConnName1 =
+                lists:concat(["replication:takeover:",
+                              binary_to_list(couch_uuids:random()), ":",
+                              OldM, "->",
+                              NewM, ":",
+                              string:slice(Bucket, 0, 60), ":",
+                              integer_to_list(VBucket)]),
+            true = length(ConnName1) =< ?MAX_DCP_CONNECTION_NAME,
+            ConnName1
+    end.
+
 do_takeover(false, Pid, _Bucket, VBucket) ->
     do_takeover(Pid, VBucket);
 do_takeover(true, Pid, Bucket, VBucket) ->
@@ -582,3 +606,87 @@ update_vbucket_map(RebalancerPid, WorkerPid, Bucket, VBucket) ->
                        [Bucket, VBucket, Error]),
             exit({failed_to_update_vbucket_map, Bucket, VBucket, Error})
     end.
+
+-ifdef(TEST).
+get_takeover_connection_name_test() ->
+    meck:new(couch_uuids, [passthrough]),
+    meck:expect(couch_uuids, random,
+                fun () -> <<"a5292f34ef9062cae8dc4a86e82ac3c8">> end),
+
+    %% Connection name fits into the maximum allowed
+
+    NodeA = 'nodeA.eng.couchbase.com',
+    NodeB = 'nodeB.eng.couchbase.com',
+    BucketAB = "bucket1",
+    ConnAB = get_takeover_connection_name(NodeA, NodeB, BucketAB, 0),
+    ?assertEqual("replication:takeover:a5292f34ef9062cae8dc4a86e82ac3c8:"
+                 "nodeB.eng.couchbase.com->nodeA.eng.couchbase.com:bucket1:0",
+                 ConnAB),
+    ?assertEqual(true, length(ConnAB) =< ?MAX_DCP_CONNECTION_NAME),
+
+    %% Test where the connection name, using the pre-NEO method, won't
+    %% fit into the maximum allowed.
+
+    Node1 = "[email protected]."
+            "couchbase-new-pxxxxxxx.svc",
+    Node2 = "[email protected]."
+            "couchbase-new-pxxxxxxx.svc",
+    Bucket12 = "com.yyyyyy.digital.ms.shoppingcart.shoppingcart.1234567890"
+               "12345678901234567890",
+    Conn12 = get_takeover_connection_name(list_to_atom(Node1),
+                                          list_to_atom(Node2),
+                                          Bucket12, 1023),
+    ?assertEqual("replication:takeover:a5292f34ef9062cae8dc4a86e82ac3c8:1."
+                 "platform-couchbase-cluster.couchb->0.platform-couchbase-"
+                 "cluster.couchb:com.yyyyyy.digital.ms.shoppingcart."
+                 "shoppingcart.123456789012:1023", Conn12),
+
+    %% Test that the node names aren't shortened too much (note the only
+    %% difference is the last character).
+
+    Node3 = "ManyManyManyManyCommonCharacters_ns_1@platform-couchbase-cluster"
+            "-0000",
+    Node4 = "ManyManyManyManyCommonCharacters_ns_1@platform-couchbase-cluster"
+            "-0001",
+    LongBucket = "travel-sample-with-a-very-very-very-very-long-bucket-name",
+    Conn34 = get_takeover_connection_name(list_to_atom(Node3),
+                                          list_to_atom(Node4),
+                                          LongBucket, 777),
+    ?assertEqual("replication:takeover:a5292f34ef9062cae8dc4a86e82ac3c8:"
+                 "s_1@platform-couchbase-cluster-0001->s_1@platform-couchbase-"
+                 "cluster-0000:travel-sample-with-a-very-very-very-very-"
+                 "long-bucket-name:777", Conn34),
+
+    %% Test with unique node names but one is much longer than the other.
+
+    Node5 = "AShortNodeName",
+    Node6 = "ManyManyManyManyCommonCharacters_ns_1@platform-couchbase-cluster"
+            "-AndEvenMoreCharactersToMakeThisNodeNameLongEnoughToRequireIt"
+            "ToBeShortened",
+    Conn56 = get_takeover_connection_name(list_to_atom(Node5),
+                                          list_to_atom(Node6),
+                                          LongBucket, 789),
+    ?assertEqual("replication:takeover:a5292f34ef9062cae8dc4a86e82ac3c8:"
+                 "ManyManyManyManyCommonCharacters_ns->AShortNodeName:"
+                 "travel-sample-with-a-very-very-very-very-long-bucket-name:"
+                 "789", Conn56),
+
+    %% Long node names with no common prefix.
+
+    Node7 = "ManyManyManyManyCommonCharacters_ns_1@platform-couchbase-cluster"
+            "-AndEvenMoreCharactersToMakeThisNodeNameLongEnoughToRequireIt"
+            "ToBeShortened",
+    Node8 = "NoCommonPrefixManyCommonCharacters_ns_1@platform-couchbase-cluster"
+            "-AndEvenMoreCharactersToMakeThisNodeNameLongEnoughToRequireIt"
+            "ToBeShortened",
+    Conn78 = get_takeover_connection_name(list_to_atom(Node7),
+                                          list_to_atom(Node8),
+                                          LongBucket, 222),
+    ?assertEqual("replication:takeover:a5292f34ef9062cae8dc4a86e82ac3c8:"
+                 "NoCommonPrefixManyCommonCharacters_->ManyManyManyMany"
+                 "CommonCharacters_ns:travel-sample-with-a-very-very-very-"
+                 "very-long-bucket-name:222", Conn78),
+
+    meck:unload(couch_uuids).
+
+-endif.