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() ->

Reply via email to