You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ii...@apache.org on 2022/02/01 18:17:53 UTC

[couchdb] branch 3.x updated: Execute fabric_rpc_tests in clean database_dir

This is an automated email from the ASF dual-hosted git repository.

iilyak pushed a commit to branch 3.x
in repository https://gitbox.apache.org/repos/asf/couchdb.git


The following commit(s) were added to refs/heads/3.x by this push:
     new b8ad56b  Execute fabric_rpc_tests in clean database_dir
     new 50080a9  Merge pull request #3921 from cloudant/fix-flaky-test-in-fabric_rpc
b8ad56b is described below

commit b8ad56b12e15d745d5ee278e18ede6d48ac6d4b1
Author: ILYA Khlopotov <ii...@apache.org>
AuthorDate: Mon Jan 31 07:47:03 2022 -0800

    Execute fabric_rpc_tests in clean database_dir
    
    The `fabric_rpc_tests` pollutes the state of `shards_db` which causes flakiness
    of other tests. This PR fixes the problem by configuring temporary `database_dir`.
    
    The important implementation detail is that we need to wait for all `couch_server`
    processes to restart. Before initroduction of sharded couch server in the
    https://github.com/apache/couchdb/pull/3366 this could be done as:
    
    ```erlang
    test_util:with_process_restart(couch_server, fun() ->
      config:set("couchdb", "database_dir", NewDatabaseDir)
    end),
    ```
    
    This method has to be updated to support sharded couch_server. Following auxilary
    functions where added:
    
    - `couch_server:names/0` - returns list of registered names of each
      `couch_server` process
    - `test_util:with_processes_restart/{2,4}` - waits all process to be restarted
      returns `{Pids :: #{} | timeout, Res :: term()}`
    - `test_util:with_couch_server_restart/1` - waits for all `couch_server` processes
    to finish restart
    
    The new way of configuring `database_dir` in test suites is:
    
    ```erlang
    test_util:with_couch_server_restart(fun() ->
      config:set("couchdb", "database_dir", NewDatabaseDir)
    end),
    ```
---
 src/couch/src/couch_server.erl             |  5 ++++
 src/couch/src/test_util.erl                | 38 ++++++++++++++++++++++++++++++
 src/fabric/test/eunit/fabric_rpc_tests.erl |  9 ++++++-
 3 files changed, 51 insertions(+), 1 deletion(-)

diff --git a/src/couch/src/couch_server.erl b/src/couch/src/couch_server.erl
index 06be867..5e29538 100644
--- a/src/couch/src/couch_server.erl
+++ b/src/couch/src/couch_server.erl
@@ -29,6 +29,7 @@
 -export([db_updated/1]).
 -export([num_servers/0, couch_server/1, couch_dbs_pid_to_name/1, couch_dbs/1]).
 -export([aggregate_queue_len/0, get_spidermonkey_version/0]).
+-export([names/0]).
 
 % config_listener api
 -export([handle_config_change/5, handle_config_terminate/3]).
@@ -995,6 +996,10 @@ aggregate_queue_len() ->
     ],
     lists:sum([X || {_, X} <- MQs]).
 
+names() ->
+    N = couch_server:num_servers(),
+    [couch_server:couch_server(I) || I <- lists:seq(1, N)].
+
 -ifdef(TEST).
 -include_lib("eunit/include/eunit.hrl").
 
diff --git a/src/couch/src/test_util.erl b/src/couch/src/test_util.erl
index 4b802bc..06dfcc6 100644
--- a/src/couch/src/test_util.erl
+++ b/src/couch/src/test_util.erl
@@ -33,6 +33,8 @@
 -export([wait_process/1, wait_process/2]).
 -export([wait/1, wait/2, wait/3]).
 -export([wait_value/2, wait_other_value/2]).
+-export([with_processes_restart/2, with_processes_restart/4]).
+-export([with_couch_server_restart/1]).
 
 -export([start/1, start/2, start/3, stop/1]).
 
@@ -90,6 +92,10 @@ stop_couch(#test_context{started = Apps}) ->
 stop_couch(_) ->
     stop_couch().
 
+with_couch_server_restart(Fun) ->
+    Servers = couch_server:names(),
+    test_util:with_processes_restart(Servers, Fun).
+
 start_applications(Apps) ->
     StartOrder = calculate_start_order(Apps),
     start_applications(StartOrder, []).
@@ -246,6 +252,38 @@ wait_other_value(Fun, Value) ->
         end
     end).
 
+with_processes_restart(Processes, Fun) ->
+    with_processes_restart(Processes, Fun, 5000, 50).
+
+with_processes_restart(Names, Fun, Timeout, Delay) ->
+    Processes = lists:foldl(
+        fun(Name, Acc) ->
+            [{Name, whereis(Name)} | Acc]
+        end,
+        [],
+        Names
+    ),
+    [catch unlink(Pid) || {_, Pid} <- Processes],
+    Res = (catch Fun()),
+    {wait_start(Processes, Timeout, Delay), Res}.
+
+wait_start(Processses, TimeoutInSec, Delay) ->
+    Now = now_us(),
+    wait_start(Processses, TimeoutInSec * 1000, Delay, Now, Now, #{}).
+
+wait_start(_, Timeout, _Delay, Started, Prev, _) when Prev - Started > Timeout ->
+    timeout;
+wait_start([], Timeout, Delay, Started, _Prev, Res) ->
+    Res;
+wait_start([{Name, Pid} | Rest] = Processes, Timeout, Delay, Started, _Prev, Res) ->
+    case whereis(Name) of
+        NewPid when is_pid(NewPid) andalso NewPid =/= Pid ->
+            wait_start(Rest, Timeout, Delay, Started, now_us(), maps:put(Name, NewPid, Res));
+        _ ->
+            ok = timer:sleep(Delay),
+            wait_start(Processes, Timeout, Delay, Started, now_us(), Res)
+    end.
+
 start(Module) ->
     start(Module, [], []).
 
diff --git a/src/fabric/test/eunit/fabric_rpc_tests.erl b/src/fabric/test/eunit/fabric_rpc_tests.erl
index 5f86128..0fc295c 100644
--- a/src/fabric/test/eunit/fabric_rpc_tests.erl
+++ b/src/fabric/test/eunit/fabric_rpc_tests.erl
@@ -54,9 +54,16 @@ main_test_() ->
     }.
 
 setup_all() ->
-    test_util:start_couch([rexi, mem3, fabric]).
+    Ctx = test_util:start_couch([rexi, mem3, fabric]),
+    DatabaseDir = config:get("couchdb", "database_dir"),
+    Suffix = ?b2l(couch_uuids:random()),
+    test_util:with_couch_server_restart(fun() ->
+        config:set("couchdb", "database_dir", DatabaseDir ++ "/" ++ Suffix, _Persist = false)
+    end),
+    Ctx.
 
 teardown_all(Ctx) ->
+    config:delete("couchdb", "database_dir", false),
     test_util:stop_couch(Ctx).
 
 setup_no_db_or_config() ->