You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by da...@apache.org on 2018/09/06 16:20:21 UTC
[couchdb] branch fix-couch-server-race-condition updated (e2df699
-> 3482db2)
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a change to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git.
discard e2df699 Reproduce race condition in couch_server
discard 47c4fc4 Fix couch_server:terminate/2
add d29011b Fixed use of find ... -printf "%f" on Mac OS by replacing it with -exec basename
add 0cbaefc added missing dependencies for Debian-based systems
add 99e801e Use new mochiweb recbuf|sndbuf undefined setting
add 54b2eaa Bump fauxton
add 0fefc85 Rename 'module' data provider to 'static_module'
add cf65280 Add 'callback_module' provider
add 7a2a390 Update documentation
add d98e3e1 Merge pull request #1432 from cloudant/support-callback-module-data-provider
add a7f2aa5 Add rexi ping message
add a6bc72e Move mango selector matching to the shard level
add 1e4f32f Ensure we only receive the correct DOWN message
add 2a3b4e7 Merge pull request #1553 from apache/ref-match-1544
add ed1db09 Fix session based replicator auth when endpoints have require_valid_user set
add 39dede7 Expose document update errors to client
add d3453d2 Merge pull request #1486 from cloudant/fix-doc-update-case-clause
add 28ba48d Reduce size of #leaf.atts keys
add 0cb1110 Add `POST /_node/$node/_restart` endpoint
add d14869e Calculate uptime since application start instead of a beam start
add fbe98b6 Use "/_node/<node>/_restart" from JavaScript tests
add 7217481 Remove special handling of 'restart' from 'test/javascript/run'
add 91ff3f6 Remove _restart endpoint from non-clustered interface
add 9be1213 Remove no longer needed handle_restart_req handler
add b847adb Merge pull request #1543 from cloudant/implement-node-restart
add 3852a00 Fix typos in couch_db_engine.erl
add da2e22a Fix bug during purge
add 151b9b0 Fix default security object handling
add bb27d5e Fix race on couch_db:reopen/1
add aea2833 Update fabric_doc_open eunit tests
add fc10fb5 Merge pull request #1366 from apache/COUCHDB-3326-clustered-purge-pr1-misc-cleanup
add 391d7b5 Simplify logic in mem3_rep
add 8819a27 Merge pull request #1570 from apache/COUCHDB-3326-clustered-purge-pr2-simplify-mem3-rep
add 9b7c2d1 Rewrite the PSE test suite to use couch_server
add 5230ca0 Move PSE tests to their own app
add d033063 Rename PSE test modules
add bbc74b9 Update PSE test definitions for new util module
add 776349c Update to use new couch_pse_tests app
add e0c7ad3 Enhance PSE tests with setup/teardown functions
add c02f6d2 Merge pull request #1368 from apache/COUCHDB-3326-clustered-purge-pr3-refactor-pse-tests
add ab12e0b Create new on_compact trigger
add 2d3f935 Merge pull request #1369 from apache/COUCHDB-3326-clustered-purge-pr4-on-compact-plugin
add febb0fc [01/10] Clustered Purge: Define new purge API
add eca282e [02/10] Clustered Purge: Update single node APIs
add bccc39d [03/10] Clustered Purge: Update couch_bt_engine
add 8a9112f [04/10] Clustered Purge: Update couch_pse_tests
add 48378a9 [05/10] Clustered Purge: Add upgrade tests
add 3be455b [06/10] Clustered Purge: Update mrview indexes
add c625933 [07/10] Clustered Purge: Internal replication
add 0234679 [08/10] Clustered Purge: Update read-repair
add 932dd93 [09/10] Clustered Purge: Fabric API
add 73ac8d1 [10/10] Clustered Purge: Clustered HTTP API
add f82b156 Merge pull request #1370 from apache/COUCHDB-3326-clustered-purge-pr5-implementation
add 2350cd4 Switch rexi_sup restart strategy to rest_for_one
add d2d2261 Fix builtin _sum reduce function
add b648bbe Fix make warning from cpse_test_purge_seqs.erl
add ee7e700 Merge pull request #1576 from apache/1573-export-all-for-pse-test
add bc03795 Improve validation of database creation parameters
add fc92fbe Merge pull request #1582 from apache/improve-dbcreate-validation
add e748d13 Fix dialyzer warning of shard record construction
add 7d984ad Merge pull request #1581 from apache/1580-shard-record-violation
add 0c6aa8b Improve cleanup_index_files
add 50be214 Merge pull request #1586 from cloudant/improve-cleanup_index_files
add dff0b97 Implement convinience `mem3:ping/2` function
add cf7e92d Merge pull request #1590 from cloudant/add-mem3-ping
add b2b6988 Check if db exists in /db/_ensure_full_commit call (#1588)
add cba1a90 Create shard files if missing
add 32ff5f7 Merge pull request #1591 from apache/create-shards-if-missing
add 1a75eec Log warning when changes seq rewinds to 0
add 485e9ca Merge pull request #1568 from cloudant/log-changes-rewind-reasons
new 7a469db Fix couch_server:terminate/2
new 690353c Reproduce race condition in couch_server
new 3482db2 Fix couch_server concurrency error
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (e2df699)
\
N -- N -- N refs/heads/fix-couch-server-race-condition (3482db2)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
INSTALL.Unix.md | 3 +
Makefile | 4 +-
rebar.config.script | 3 +-
rel/overlay/etc/default.ini | 8 +-
rel/overlay/etc/local.ini | 2 +-
src/chttpd/src/chttpd_db.erl | 65 +-
src/chttpd/src/chttpd_misc.erl | 8 +-
src/chttpd/test/chttpd_db_test.erl | 22 +
src/chttpd/test/chttpd_purge_tests.erl | 320 ++++++++++
src/couch/priv/stats_descriptions.cfg | 12 +
src/couch/src/couch_app.erl | 11 +-
src/couch/src/couch_att.erl | 10 +-
src/couch/src/couch_bt_engine.erl | 253 ++++++--
src/couch/src/couch_bt_engine.hrl | 4 +-
src/couch/src/couch_bt_engine_compactor.erl | 124 +++-
src/couch/src/couch_bt_engine_header.erl | 35 +-
src/couch/src/couch_db.erl | 175 ++++-
src/couch/src/couch_db_engine.erl | 206 ++++--
src/couch/src/couch_db_plugin.erl | 11 +
src/couch/src/couch_db_updater.erl | 185 +++---
src/couch/src/couch_httpd_db.erl | 23 +-
src/couch/src/couch_httpd_misc_handlers.erl | 23 +-
src/couch/src/couch_query_servers.erl | 27 +-
src/couch/src/couch_server.erl | 28 +-
src/couch/src/test_engine_compaction.erl | 185 ------
src/couch/src/test_engine_fold_changes.erl | 190 ------
src/couch/src/test_engine_get_set_props.erl | 70 --
src/couch/src/test_engine_open_close_delete.erl | 81 ---
src/couch/src/test_engine_purge_docs.erl | 158 -----
src/couch/src/test_engine_read_write_docs.erl | 317 ----------
src/couch/src/test_engine_util.erl | 608 ------------------
src/couch/test/couch_bt_engine_tests.erl | 2 +-
src/couch/test/couch_bt_engine_upgrade_tests.erl | 220 +++++++
src/couch/test/couch_query_servers_tests.erl | 95 +++
src/couch/test/couch_server_tests.erl | 110 ++--
src/couch/test/couchdb_mrview_tests.erl | 53 ++
.../{test.couch => db_with_1_purge_req.couch} | Bin 16482 -> 12470 bytes
....couch => db_with_1_purge_req_for_2_docs.couch} | Bin 16482 -> 16557 bytes
.../{test.couch => db_with_2_purge_req.couch} | Bin 16482 -> 16566 bytes
src/couch/test/fixtures/db_without_purge_req.couch | Bin 0 -> 61644 bytes
src/couch_epi/README.md | 35 +-
src/couch_epi/src/couch_epi_data.erl | 11 +-
src/couch_epi/src/couch_epi_sup.erl | 5 +-
src/couch_epi/test/couch_epi_tests.erl | 67 +-
src/couch_index/src/couch_index_epi.erl | 5 +-
.../src/couch_index_plugin_couch_db.erl} | 25 +-
src/couch_index/src/couch_index_updater.erl | 50 +-
src/couch_mrview/src/couch_mrview_cleanup.erl | 16 +-
src/couch_mrview/src/couch_mrview_index.erl | 115 +++-
src/couch_mrview/src/couch_mrview_show.erl | 4 +-
src/couch_mrview/src/couch_mrview_test_util.erl | 5 +
src/couch_mrview/src/couch_mrview_updater.erl | 14 +-
src/couch_mrview/src/couch_mrview_util.erl | 39 +-
.../test/couch_mrview_purge_docs_fabric_tests.erl | 276 ++++++++
.../test/couch_mrview_purge_docs_tests.erl | 506 +++++++++++++++
.../src/couch_pse_tests.app.src} | 11 +-
src/couch_pse_tests/src/cpse_gather.erl | 95 +++
.../src/cpse_test_attachments.erl} | 50 +-
src/couch_pse_tests/src/cpse_test_compaction.erl | 318 ++++++++++
src/couch_pse_tests/src/cpse_test_fold_changes.erl | 198 ++++++
.../src/cpse_test_fold_docs.erl} | 167 ++---
.../src/cpse_test_fold_purge_infos.erl | 166 +++++
.../src/cpse_test_get_set_props.erl | 95 +++
.../src/cpse_test_open_close_delete.erl | 76 +++
.../src/cpse_test_purge_bad_checkpoints.erl | 80 +++
src/couch_pse_tests/src/cpse_test_purge_docs.erl | 463 ++++++++++++++
.../src/cpse_test_purge_replication.erl | 202 ++++++
src/couch_pse_tests/src/cpse_test_purge_seqs.erl | 126 ++++
.../src/cpse_test_read_write_docs.erl | 318 ++++++++++
.../src/cpse_test_ref_counting.erl} | 57 +-
src/couch_pse_tests/src/cpse_util.erl | 674 ++++++++++++++++++++
.../src/couch_replicator_auth_session.erl | 90 ++-
src/fabric/src/fabric.erl | 91 ++-
src/fabric/src/fabric_db_info.erl | 29 +-
src/fabric/src/fabric_db_meta.erl | 26 +-
src/fabric/src/fabric_doc_open.erl | 702 +++++++++++----------
src/fabric/src/fabric_doc_open_revs.erl | 262 ++++++--
src/fabric/src/fabric_doc_purge.erl | 572 +++++++++++++++++
src/fabric/src/fabric_doc_update.erl | 3 +-
src/fabric/src/fabric_rpc.erl | 147 ++++-
src/fabric/src/fabric_util.erl | 2 +-
src/fabric/src/fabric_view_changes.erl | 10 +-
src/fabric/test/fabric_rpc_purge_tests.erl | 285 +++++++++
src/mango/src/mango_cursor_view.erl | 112 +++-
src/mango/src/mango_execution_stats.erl | 7 +-
src/mango/test/20-no-timeout-test.py | 38 ++
src/mem3/include/mem3.hrl | 12 +-
src/mem3/src/mem3.erl | 26 +-
src/mem3/src/mem3_epi.erl | 3 +-
.../src/mem3_plugin_couch_db.erl} | 8 +-
src/mem3/src/mem3_rep.erl | 228 ++++++-
src/mem3/src/mem3_rpc.erl | 71 ++-
src/mem3/src/mem3_util.erl | 15 +-
src/rexi/src/rexi.erl | 9 +
src/rexi/src/rexi_sup.erl | 2 +-
src/rexi/src/rexi_utils.erl | 2 +
test/javascript/run | 6 +-
test/javascript/test_setup.js | 11 +-
test/javascript/tests/erlang_views.js | 5 +-
test/javascript/tests/purge.js | 27 +-
100 files changed, 8094 insertions(+), 2627 deletions(-)
create mode 100644 src/chttpd/test/chttpd_purge_tests.erl
delete mode 100644 src/couch/src/test_engine_compaction.erl
delete mode 100644 src/couch/src/test_engine_fold_changes.erl
delete mode 100644 src/couch/src/test_engine_get_set_props.erl
delete mode 100644 src/couch/src/test_engine_open_close_delete.erl
delete mode 100644 src/couch/src/test_engine_purge_docs.erl
delete mode 100644 src/couch/src/test_engine_read_write_docs.erl
delete mode 100644 src/couch/src/test_engine_util.erl
create mode 100644 src/couch/test/couch_bt_engine_upgrade_tests.erl
create mode 100644 src/couch/test/couch_query_servers_tests.erl
copy src/couch/test/fixtures/{test.couch => db_with_1_purge_req.couch} (66%)
copy src/couch/test/fixtures/{test.couch => db_with_1_purge_req_for_2_docs.couch} (83%)
copy src/couch/test/fixtures/{test.couch => db_with_2_purge_req.couch} (86%)
create mode 100644 src/couch/test/fixtures/db_without_purge_req.couch
copy src/{ddoc_cache/src/ddoc_cache_entry_custom.erl => couch_index/src/couch_index_plugin_couch_db.erl} (67%)
create mode 100644 src/couch_mrview/test/couch_mrview_purge_docs_fabric_tests.erl
create mode 100644 src/couch_mrview/test/couch_mrview_purge_docs_tests.erl
copy src/{couch_log/src/couch_log.app.src => couch_pse_tests/src/couch_pse_tests.app.src} (76%)
create mode 100644 src/couch_pse_tests/src/cpse_gather.erl
rename src/{couch/src/test_engine_attachments.erl => couch_pse_tests/src/cpse_test_attachments.erl} (63%)
create mode 100644 src/couch_pse_tests/src/cpse_test_compaction.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_fold_changes.erl
rename src/{couch/src/test_engine_fold_docs.erl => couch_pse_tests/src/cpse_test_fold_docs.erl} (62%)
create mode 100644 src/couch_pse_tests/src/cpse_test_fold_purge_infos.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_get_set_props.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_open_close_delete.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_purge_bad_checkpoints.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_purge_docs.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_purge_replication.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_purge_seqs.erl
create mode 100644 src/couch_pse_tests/src/cpse_test_read_write_docs.erl
rename src/{couch/src/test_engine_ref_counting.erl => couch_pse_tests/src/cpse_test_ref_counting.erl} (62%)
create mode 100644 src/couch_pse_tests/src/cpse_util.erl
create mode 100644 src/fabric/src/fabric_doc_purge.erl
create mode 100644 src/fabric/test/fabric_rpc_purge_tests.erl
create mode 100644 src/mango/test/20-no-timeout-test.py
copy src/{ddoc_cache/test/ddoc_cache_ev.erl => mem3/src/mem3_plugin_couch_db.erl} (78%)
[couchdb] 01/03: Fix couch_server:terminate/2
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 7a469db180db6073b2163cf2cb4e8661c7bf39c9
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Wed Sep 5 16:24:17 2018 -0500
Fix couch_server:terminate/2
If couch_server terminates while there is an active open_async process
it will throw a function_clause exception because `couch_db:get_pid/1`
will fail due to the `#entry.db` member being undefined. Simple fix is
to just filter those out.
---
src/couch/src/couch_server.erl | 6 +++++-
1 file changed, 5 insertions(+), 1 deletion(-)
diff --git a/src/couch/src/couch_server.erl b/src/couch/src/couch_server.erl
index ede8227..af728fa 100644
--- a/src/couch/src/couch_server.erl
+++ b/src/couch/src/couch_server.erl
@@ -244,7 +244,11 @@ terminate(Reason, Srv) ->
[Reason,
Srv#server{lru = redacted}]),
ets:foldl(fun(#entry{db = Db}, _) ->
- couch_util:shutdown_sync(couch_db:get_pid(Db))
+ % Filter out any entry records for open_async
+ % processes that haven't finished.
+ if Db == undefined -> ok; true ->
+ couch_util:shutdown_sync(couch_db:get_pid(Db))
+ end
end, nil, couch_dbs),
ok.
[couchdb] 02/03: Reproduce race condition in couch_server
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 690353c20ddae6e4b280eadfa862813764c60ab8
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Wed Sep 5 16:25:41 2018 -0500
Reproduce race condition in couch_server
A rather uncommon bug found in production. Will write more as this is
just for show and tell.
For now this test case just demonstrates the issue that was discovered.
A fix is still being pondered.
---
src/couch/test/couch_server_tests.erl | 173 ++++++++++++++++++++++++++++++++++
1 file changed, 173 insertions(+)
diff --git a/src/couch/test/couch_server_tests.erl b/src/couch/test/couch_server_tests.erl
index a79f20f..f2a98e7 100644
--- a/src/couch/test/couch_server_tests.erl
+++ b/src/couch/test/couch_server_tests.erl
@@ -14,6 +14,7 @@
-include_lib("couch/include/couch_eunit.hrl").
-include_lib("couch/include/couch_db.hrl").
+-include("../src/couch_server_int.hrl").
start() ->
Ctx = test_util:start_couch(),
@@ -105,3 +106,175 @@ bad_engine_option_test_() ->
t_bad_engine_option() ->
Resp = couch_server:create(?tempdb(), [{engine, <<"cowabunga!">>}]),
?assertEqual(Resp, {error, {invalid_engine_extension, <<"cowabunga!">>}}).
+
+
+interleaved_requests_test_() ->
+ {
+ setup,
+ fun start_interleaved/0,
+ fun stop_interleaved/1,
+ fun make_interleaved_requests/1
+ }.
+
+
+start_interleaved() ->
+ TestDbName = ?tempdb(),
+ meck:new(couch_db, [passthrough]),
+ meck:expect(couch_db, start_link, fun(Engine, DbName, Filename, Options) ->
+ case DbName of
+ TestDbName ->
+ receive
+ go -> ok
+ end,
+ Res = meck:passthrough([Engine, DbName, Filename, Options]),
+ % We're unlinking and sending a delayed
+ % EXIT signal so that we can mimic a specific
+ % message order in couch_server. On a test machine
+ % this is a big race condition which affects the
+ % ability to induce the bug.
+ case Res of
+ {ok, Db} ->
+ DbPid = couch_db:get_pid(Db),
+ unlink(DbPid),
+ Msg = {'EXIT', DbPid, killed},
+ erlang:send_after(2000, whereis(couch_server), Msg);
+ _ ->
+ ok
+ end,
+ Res;
+ _ ->
+ meck:passthrough([Engine, DbName, Filename, Options])
+ end
+ end),
+ {test_util:start_couch(), TestDbName}.
+
+
+stop_interleaved({Ctx, TestDbName}) ->
+ couch_server:delete(TestDbName, [?ADMIN_CTX]),
+ meck:unload(),
+ test_util:stop_couch(Ctx).
+
+
+make_interleaved_requests({_, TestDbName}) ->
+ [
+ fun() -> t_interleaved_create_delete_open(TestDbName) end
+ ].
+
+
+t_interleaved_create_delete_open(DbName) ->
+ {CrtRef, DelRef, OpenRef} = {make_ref(), make_ref(), make_ref()},
+ CrtMsg = {'$gen_call', {self(), CrtRef}, {create, DbName, [?ADMIN_CTX]}},
+ DelMsg = {'$gen_call', {self(), DelRef}, {delete, DbName, [?ADMIN_CTX]}},
+ OpenMsg = {'$gen_call', {self(), OpenRef}, {open, DbName, [?ADMIN_CTX]}},
+
+ % Get the current couch_server pid so we're sure
+ % to not end up messaging two different pids
+ CouchServer = whereis(couch_server),
+
+ % Start our first instance that will succeed in
+ % an invalid state. Notice that the opener pid
+ % spawned by couch_server:open_async/5 will halt
+ % in our meck expect function waiting for a message.
+ %
+ % We're using raw message passing here so that we don't
+ % have to coordinate multiple processes for this test.
+ CouchServer ! CrtMsg,
+ {ok, Opener} = get_opener_pid(DbName),
+
+ % We have to suspend couch_server so that we can enqueue
+ % our next requests and let the opener finish processing.
+ erlang:suspend_process(CouchServer),
+
+ % Since couch_server is suspend, this delete request won't
+ % be processed until after the opener has sent its
+ % successful open response via gen_server:call/3
+ CouchServer ! DelMsg,
+
+ % This open request will be in the queue after the
+ % delete request but before the gen_server:call/3
+ % message which will establish the mixed up state
+ % in the couch_dbs ets table
+ CouchServer ! OpenMsg,
+
+ % First release the opener pid so it can continue
+ % working while we tweak meck
+ Opener ! go,
+
+ % Replace our expect call to meck so that the OpenMsg
+ % isn't blocked on the receive
+ meck:expect(couch_db, start_link, fun(Engine, DbName1, Filename, Options) ->
+ meck:passthrough([Engine, DbName1, Filename, Options])
+ end),
+
+ % Wait for the '$gen_call' message from OpenerPid to arrive
+ % in couch_server's mailbox
+ ok = wait_for_open_async_result(CouchServer, Opener),
+
+ % Now monitor and resume the couch_server and assert that
+ % couch_server does not crash while processing OpenMsg
+ CSRef = erlang:monitor(process, CouchServer),
+ erlang:resume_process(CouchServer),
+ check_monitor_not_triggered(CSRef),
+
+ % The create response is expected to return not_found
+ % due to the delete request canceling the async opener
+ % pid and sending not_found to all waiters unconditionally
+ ?assertEqual({CrtRef, not_found}, get_next_message()),
+
+ % Our delete request was processed normally
+ ?assertEqual({DelRef, ok}, get_next_message()),
+
+ % TODO: This assertion will change after I fix the bug as
+ % its incorrectly receiving the create message's response
+ ?assertMatch({OpenRef, {ok, _}}, get_next_message()),
+
+ % And finally assert that couch_server is still
+ % alive.
+ ?assert(is_process_alive(CouchServer)),
+ check_monitor_not_triggered(CSRef).
+
+
+get_opener_pid(DbName) ->
+ WaitFun = fun() ->
+ case ets:lookup(couch_dbs, DbName) of
+ [#entry{pid = Pid}] ->
+ {ok, Pid};
+ [] ->
+ wait
+ end
+ end,
+ test_util:wait(WaitFun).
+
+
+wait_for_open_async_result(CouchServer, Opener) ->
+ WaitFun = fun() ->
+ {_, Messages} = erlang:process_info(CouchServer, messages),
+ Found = lists:foldl(fun(Msg, Acc) ->
+ case Msg of
+ {'$gen_call', {Opener, _}, {open_result, _, _, {ok, _}}} ->
+ true;
+ _ ->
+ Acc
+ end
+ end, false, Messages),
+ if Found -> ok; true -> wait end
+ end,
+ test_util:wait(WaitFun).
+
+
+check_monitor_not_triggered(Ref) ->
+ receive
+ {'DOWN', Ref, _, _, Reason0} ->
+ erlang:error({monitor_triggered, Reason0})
+ after 100 ->
+ ok
+ end.
+
+
+get_next_message() ->
+ receive
+ Msg ->
+ Msg
+ after 5000 ->
+ erlang:error(timeout)
+ end.
[couchdb] 03/03: Fix couch_server concurrency error
Posted by da...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch fix-couch-server-race-condition
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 3482db2f96b7e9c76e67c568e68080072a07b4cc
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Thu Sep 6 11:01:07 2018 -0500
Fix couch_server concurrency error
Its possible that a busy couch_server and a specific ordering and timing
of events can end up with an open_async message in the mailbox while a
new and unrelated open_async process is spawned. This change just ensure
that if we encounter any old messages in the mailbox that we ignore
them.
The underlying issue here is that a delete request clears out the state
in our couch_dbs ets table while not clearing out state in the message
queue. In some fairly specific circumstances this leads to the message
on in the mailbox satisfying an ets entry for a newer open_async
process. This change just includes a match on the opener process.
Anything unmatched came before the current open_async request which
means it should be ignored.
---
src/couch/src/couch_server.erl | 26 ++++++++++++++++++--------
src/couch/test/couch_server_tests.erl | 6 +++---
2 files changed, 21 insertions(+), 11 deletions(-)
diff --git a/src/couch/src/couch_server.erl b/src/couch/src/couch_server.erl
index af728fa..46000ed 100644
--- a/src/couch/src/couch_server.erl
+++ b/src/couch/src/couch_server.erl
@@ -390,8 +390,8 @@ handle_call(reload_engines, _From, Server) ->
{reply, ok, Server#server{engines = get_configured_engines()}};
handle_call(get_server, _From, Server) ->
{reply, {ok, Server}, Server};
-handle_call({open_result, T0, DbName, {ok, Db}}, {FromPid, _Tag}, Server) ->
- true = ets:delete(couch_dbs_pid_to_name, FromPid),
+handle_call({open_result, T0, DbName, {ok, Db}}, {Opener, _}, Server) ->
+ true = ets:delete(couch_dbs_pid_to_name, Opener),
OpenTime = timer:now_diff(os:timestamp(), T0) / 1000,
couch_stats:update_histogram([couchdb, db_open_time], OpenTime),
DbPid = couch_db:get_pid(Db),
@@ -400,7 +400,7 @@ handle_call({open_result, T0, DbName, {ok, Db}}, {FromPid, _Tag}, Server) ->
% db was deleted during async open
exit(DbPid, kill),
{reply, ok, Server};
- [#entry{req_type = ReqType, waiters = Waiters} = Entry] ->
+ [#entry{pid = Opener, req_type = ReqType, waiters = Waiters} = Entry] ->
link(DbPid),
[gen_server:reply(Waiter, {ok, Db}) || Waiter <- Waiters],
% Cancel the creation request if it exists.
@@ -425,27 +425,37 @@ handle_call({open_result, T0, DbName, {ok, Db}}, {FromPid, _Tag}, Server) ->
true ->
Server#server.lru
end,
- {reply, ok, Server#server{lru = Lru}}
+ {reply, ok, Server#server{lru = Lru}};
+ [#entry{}] ->
+ % A mismatched opener pid means that this open_result message
+ % was in our mailbox but is now stale. Mostly ignore
+ % it except to ensure that the db pid is super dead.
+ exit(couch_db:get_pid(Db), kill)
+ {reply, ok, Server}
end;
handle_call({open_result, T0, DbName, {error, eexist}}, From, Server) ->
handle_call({open_result, T0, DbName, file_exists}, From, Server);
-handle_call({open_result, _T0, DbName, Error}, {FromPid, _Tag}, Server) ->
+handle_call({open_result, _T0, DbName, Error}, {Opener, _}, Server) ->
case ets:lookup(couch_dbs, DbName) of
[] ->
% db was deleted during async open
{reply, ok, Server};
- [#entry{req_type = ReqType, waiters = Waiters} = Entry] ->
+ [#entry{pid = Opener, req_type = ReqType, waiters = Waiters} = Entry] ->
[gen_server:reply(Waiter, Error) || Waiter <- Waiters],
couch_log:info("open_result error ~p for ~s", [Error, DbName]),
true = ets:delete(couch_dbs, DbName),
- true = ets:delete(couch_dbs_pid_to_name, FromPid),
+ true = ets:delete(couch_dbs_pid_to_name, Opener),
NewServer = case ReqType of
{create, DbName, Engine, Options, CrFrom} ->
open_async(Server, CrFrom, DbName, Engine, Options);
_ ->
Server
end,
- {reply, ok, db_closed(NewServer, Entry#entry.db_options)}
+ {reply, ok, db_closed(NewServer, Entry#entry.db_options)};
+ [#entry{}] ->
+ % A mismatched pid means that this open_result message
+ % was in our mailbox and is now stale. Ignore it.
+ {reply, ok, Server}
end;
handle_call({open, DbName, Options}, From, Server) ->
case ets:lookup(couch_dbs, DbName) of
diff --git a/src/couch/test/couch_server_tests.erl b/src/couch/test/couch_server_tests.erl
index f2a98e7..3701774 100644
--- a/src/couch/test/couch_server_tests.erl
+++ b/src/couch/test/couch_server_tests.erl
@@ -224,9 +224,9 @@ t_interleaved_create_delete_open(DbName) ->
% Our delete request was processed normally
?assertEqual({DelRef, ok}, get_next_message()),
- % TODO: This assertion will change after I fix the bug as
- % its incorrectly receiving the create message's response
- ?assertMatch({OpenRef, {ok, _}}, get_next_message()),
+ % The db was deleted thus it should be not found
+ % when we try and open it.
+ ?assertMatch({OpenRef, not_found}, get_next_message()),
% And finally assert that couch_server is still
% alive.