Repository: couchdb-couch Updated Branches: refs/heads/master 100241c18 -> 3fed6863a
Disable should_process_waiting_queue_as_fifo test COUCHDB-3140 This PR disables the problematic queue-as-fifo test. As written, any call in a test to spawn_client/1 returns immediately, and does not guarantee that the call to couch_query_servers:get_ddoc_process/2 returns before control flow proceeds to the next line in the eunit test. As it turns out, on Windows, the call to spawn_client for ddoc5 succeeds prior to the ddoc4 call in this test, thus always failing. Insertion of a timer:sleep/1 call between the spawn_client/1 calls seems to solve the problem, but even this is "lucky" behaviour. Semantically the way this test is written, with spawn_client/1 using spawn/1 to spin off the actual get_ddoc_process/2 call, there is no guarantee that the couch_proc_manager FIFO queue is being populated correctly. Further, if the get_ddoc_process/2 call takes longer than the defined timeout (5000 ms) this test will always fail, as no provision is made to keep trying for an os_process after that time. Again we are "lucky" that the test runs fast enough that this is not a problem, but relying on a performant BEAM VM for test success in any regard is equally problematic. For this reason I am completely commenting out this test until the harness can be fixed. I believe the impact to couchdb is minimal. (It also suggests we may want to rethink how requests for os processes are queued and issued at some point in the future; this approach seems sub-optimal, especially if strict FIFO ordering is an expectation of the system.) /cc @eiri @rnewson @janl Project: http://git-wip-us.apache.org/repos/asf/couchdb-couch/repo Commit: http://git-wip-us.apache.org/repos/asf/couchdb-couch/commit/ac7435c3 Tree: http://git-wip-us.apache.org/repos/asf/couchdb-couch/tree/ac7435c3 Diff: http://git-wip-us.apache.org/repos/asf/couchdb-couch/diff/ac7435c3 Branch: refs/heads/master Commit: ac7435c334eae2b8454fa6d0b6a1154bca8f6874 Parents: 100241c Author: Joan Touzet <woh...@atypical.net> Authored: Sun Sep 11 02:24:40 2016 -0400 Committer: Joan Touzet <woh...@atypical.net> Committed: Sun Sep 11 02:24:40 2016 -0400 ---------------------------------------------------------------------- test/couchdb_os_proc_pool.erl | 59 +++++++++++++++++++------------------- 1 file changed, 30 insertions(+), 29 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/couchdb-couch/blob/ac7435c3/test/couchdb_os_proc_pool.erl ---------------------------------------------------------------------- diff --git a/test/couchdb_os_proc_pool.erl b/test/couchdb_os_proc_pool.erl index 54d19f0..f14af68 100644 --- a/test/couchdb_os_proc_pool.erl +++ b/test/couchdb_os_proc_pool.erl @@ -38,7 +38,7 @@ os_proc_pool_test_() -> should_block_new_proc_on_full_pool(), should_free_slot_on_proc_unexpected_exit(), should_reuse_known_proc(), - should_process_waiting_queue_as_fifo(), +% should_process_waiting_queue_as_fifo(), should_reduce_pool_on_idle_os_procs() ] } @@ -148,34 +148,35 @@ should_reuse_known_proc() -> end). -should_process_waiting_queue_as_fifo() -> - ?_test(begin - Client1 = spawn_client(<<"ddoc1">>), - Client2 = spawn_client(<<"ddoc2">>), - Client3 = spawn_client(<<"ddoc3">>), - Client4 = spawn_client(<<"ddoc4">>), - Client5 = spawn_client(<<"ddoc5">>), - - ?assertEqual(ok, ping_client(Client1)), - ?assertEqual(ok, ping_client(Client2)), - ?assertEqual(ok, ping_client(Client3)), - ?assertEqual(timeout, ping_client(Client4)), - ?assertEqual(timeout, ping_client(Client5)), - - Proc1 = get_client_proc(Client1, "1"), - ?assertEqual(ok, stop_client(Client1)), - ?assertEqual(ok, ping_client(Client4)), - Proc4 = get_client_proc(Client4, "4"), - - ?assertNotEqual(Proc4#proc.client, Proc1#proc.client), - ?assertEqual(Proc1#proc.pid, Proc4#proc.pid), - ?assertEqual(timeout, ping_client(Client5)), - - ?assertEqual(ok, stop_client(Client2)), - ?assertEqual(ok, stop_client(Client3)), - ?assertEqual(ok, stop_client(Client4)), - ?assertEqual(ok, stop_client(Client5)) - end). +%should_process_waiting_queue_as_fifo() -> +% ?_test(begin +% Client1 = spawn_client(<<"ddoc1">>), +% Client2 = spawn_client(<<"ddoc2">>), +% Client3 = spawn_client(<<"ddoc3">>), +% Client4 = spawn_client(<<"ddoc4">>), +% timer:sleep(100), +% Client5 = spawn_client(<<"ddoc5">>), +% +% ?assertEqual(ok, ping_client(Client1)), +% ?assertEqual(ok, ping_client(Client2)), +% ?assertEqual(ok, ping_client(Client3)), +% ?assertEqual(timeout, ping_client(Client4)), +% ?assertEqual(timeout, ping_client(Client5)), +% +% Proc1 = get_client_proc(Client1, "1"), +% ?assertEqual(ok, stop_client(Client1)), +% ?assertEqual(ok, ping_client(Client4)), +% Proc4 = get_client_proc(Client4, "4"), +% +% ?assertNotEqual(Proc4#proc.client, Proc1#proc.client), +% ?assertEqual(Proc1#proc.pid, Proc4#proc.pid), +% ?assertEqual(timeout, ping_client(Client5)), +% +% ?assertEqual(ok, stop_client(Client2)), +% ?assertEqual(ok, stop_client(Client3)), +% ?assertEqual(ok, stop_client(Client4)), +% ?assertEqual(ok, stop_client(Client5)) +% end). should_reduce_pool_on_idle_os_procs() ->
