Merge pull request #10666 from fix/EMQX-9573/testcase

fix(ft): anticipate repeated `kickoff`s + fix testcase

apps/emqx_ft/src/emqx_ft_assembler.erl

@@ -78,6 +78,8 @@ handle_event(info, kickoff, idle, St) ->
     % We could wait for this message and handle it at the end of the assembling rather than at
     % the beginning, however it would make error handling much more messier.
     {next_state, list_local_fragments, St, ?internal([])};
+handle_event(info, kickoff, _, _St) ->
+    keep_state_and_data;
 handle_event(
     internal,
     _,

apps/emqx_ft/test/emqx_ft_SUITE.erl

@@ -37,15 +37,27 @@ all() ->
 
 groups() ->
     [
-        {single_node, [], emqx_common_test_helpers:all(?MODULE) -- group_cluster()},
+        {single_node, [parallel], [
-        {cluster, [], group_cluster()}
+            t_assemble_crash,
-    ].
+            t_corrupted_segment_retry,
-
+            t_invalid_checksum,
-group_cluster() ->
+            t_invalid_fileid,
-    [
+            t_invalid_filename,
-        t_switch_node,
+            t_invalid_meta,
-        t_unreliable_migrating_client,
+            t_invalid_topic_format,
-        t_concurrent_fins
+            t_meta_conflict,
+            t_nasty_clientids_fileids,
+            t_no_meta,
+            t_no_segment,
+            t_simple_transfer
+        ]},
+        {cluster, [], [
+            t_switch_node,
+            t_unreliable_migrating_client,
+            {g_concurrent_fins, [{repeat_until_any_fail, 8}], [
+                t_concurrent_fins
+            ]}
+        ]}
     ].
 
 init_per_suite(Config) ->
@@ -563,10 +575,15 @@ t_unreliable_migrating_client(Config) ->
     ].
 
 t_concurrent_fins(Config) ->
+    ct:timetrap({seconds, 10}),
+
     NodeSelf = node(),
     [Node1, Node2] = ?config(cluster_nodes, Config),
 
-    ClientId = ?config(clientid, Config),
+    ClientId = iolist_to_binary([
+        ?config(clientid, Config),
+        integer_to_list(erlang:unique_integer())
+    ]),
     FileId = emqx_guid:to_hexstr(emqx_guid:gen()),
     Filename = "migratory-birds-in-southern-hemisphere-2013.pdf",
     Filesize = 100,
@@ -593,46 +610,52 @@ t_concurrent_fins(Config) ->
     ),
 
     %% Now send fins concurrently to the 3 nodes
-    Self = self(),
     Nodes = [Node1, Node2, NodeSelf],
-    FinSenders = lists:map(
+    SendFin = fun(Node) ->
+        run_commands(
+            [
+                {fun connect_mqtt_client/2, [Node]},
+                {fun send_finish/1, []}
+            ],
+            Context1
+        )
+    end,
+
+    PidMons = lists:map(
         fun(Node) ->
-            %% takeovers and disconnects will happen due to concurrency
+            erlang:spawn_monitor(fun F() ->
-            _ = erlang:process_flag(trap_exit, true),
+                _ = erlang:process_flag(trap_exit, true),
-            _Context = run_commands(
+                try
-                [
+                    SendFin(Node)
-                    {fun connect_mqtt_client/2, [Node]},
+                catch
-                    {fun send_finish/1, []}
+                    C:E ->
-                ],
+                        % NOTE: random delay to avoid livelock conditions
-                Context1
+                        ct:pal("Node ~p did not send finish successfully: ~p:~p", [Node, C, E]),
-            ),
+                        ok = timer:sleep(rand:uniform(10)),
-            Self ! {done, Node}
+                        F()
+                end
+            end)
         end,
         Nodes
     ),
     ok = lists:foreach(
-        fun(F) ->
+        fun({Pid, MRef}) ->
-            _Pid = spawn_link(F)
-        end,
-        FinSenders
-    ),
-    ok = lists:foreach(
-        fun(Node) ->
             receive
-                {done, Node} -> ok
+                {'DOWN', MRef, process, Pid, normal} -> ok
-            after 1000 ->
-                ct:fail("Node ~p did not send finish successfully", [Node])
             end
         end,
-        Nodes
+        PidMons
     ),
 
     %% Only one node should have the file
-    Exports = list_files(?config(clientid, Config)),
+    Exports = list_files(ClientId),
-    ?assertMatch(
+    case fs_exported_file_attributes(Exports) of
-        [#{"node" := _Node}],
+        [#{"node" := _Node}] ->
-        fs_exported_file_attributes(Exports)
+            ok;
-    ).
+        [#{"node" := _Node} | _] = Files ->
+            % ...But we can't really guarantee that
+            ct:comment({multiple_files_on_different_nodes, Files})
+    end.
 
 %%------------------------------------------------------------------------------
 %% Command helpers