You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by fd...@apache.org on 2011/06/21 12:13:34 UTC

svn commit: r1137928 - in /couchdb/trunk: share/www/script/test/replicator_db.js src/couchdb/couch_replication_manager.erl src/couchdb/couch_replicator_utils.erl

Author: fdmanana
Date: Tue Jun 21 10:13:33 2011
New Revision: 1137928

URL: http://svn.apache.org/viewvc?rev=1137928&view=rev
Log:
Fix server crash associated with the replicator database

If there's an exception when calculating the replication ID
for a replication document, it crashes the replication manager
gen_server 10 times. 10 is the maximum number of restarts per
hour specified for the couch_server_sup supervisor.
An easy way to trigger such exception is to specify a non
existent filter in a replication document.

This closes COUCHDB-1199.


Modified:
    couchdb/trunk/share/www/script/test/replicator_db.js
    couchdb/trunk/src/couchdb/couch_replication_manager.erl
    couchdb/trunk/src/couchdb/couch_replicator_utils.erl

Modified: couchdb/trunk/share/www/script/test/replicator_db.js
URL: http://svn.apache.org/viewvc/couchdb/trunk/share/www/script/test/replicator_db.js?rev=1137928&r1=1137927&r2=1137928&view=diff
==============================================================================
--- couchdb/trunk/share/www/script/test/replicator_db.js (original)
+++ couchdb/trunk/share/www/script/test/replicator_db.js Tue Jun 21 10:13:33 2011
@@ -1279,6 +1279,68 @@ couchTests.replicator_db = function(debu
   }
 
 
+  function test_invalid_filter() {
+    // COUCHDB-1199 - replication document with a filter field that was invalid
+    // crashed the CouchDB server.
+    var repDoc1 = {
+       _id: "rep1",
+       source: "couch_foo_test_db",
+       target: "couch_bar_test_db",
+       filter: "test/foofilter"
+    };
+
+    TEquals(true, repDb.save(repDoc1).ok);
+
+    waitForRep(repDb, repDoc1, "error");
+    repDoc1 = repDb.open(repDoc1._id);
+    TEquals("undefined", typeof repDoc1._replication_id);
+    TEquals("error", repDoc1._replication_state);
+
+    populate_db(dbA, docs1);
+    populate_db(dbB, []);
+
+    var repDoc2 = {
+       _id: "rep2",
+       source: dbA.name,
+       target: dbB.name,
+       filter: "test/foofilter"
+    };
+
+    TEquals(true, repDb.save(repDoc2).ok);
+
+    waitForRep(repDb, repDoc2, "error");
+    repDoc2 = repDb.open(repDoc2._id);
+    TEquals("undefined", typeof repDoc2._replication_id);
+    TEquals("error", repDoc2._replication_state);
+
+    var ddoc = {
+      _id: "_design/mydesign",
+      language : "javascript",
+      filters : {
+        myfilter : (function(doc, req) {
+          return true;
+        }).toString()
+      }
+    };
+
+    TEquals(true, dbA.save(ddoc).ok);
+
+    var repDoc3 = {
+       _id: "rep3",
+       source: dbA.name,
+       target: dbB.name,
+       filter: "mydesign/myfilter"
+    };
+
+    TEquals(true, repDb.save(repDoc3).ok);
+
+    waitForRep(repDb, repDoc3, "completed");
+    repDoc3 = repDb.open(repDoc3._id);
+    TEquals("string", typeof repDoc3._replication_id);
+    TEquals("completed", repDoc3._replication_state);
+  }
+
+
   // run all the tests
   var server_config = [
     {
@@ -1355,6 +1417,11 @@ couchTests.replicator_db = function(debu
   restartServer();
   run_on_modified_server(server_config, rep_doc_field_validation);
 
+
+  repDb.deleteDb();
+  restartServer();
+  run_on_modified_server(server_config, test_invalid_filter);
+
 /*
  * Disabled, since error state would be set on the document only after
  * the exponential backoff retry done by the replicator database listener

Modified: couchdb/trunk/src/couchdb/couch_replication_manager.erl
URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_replication_manager.erl?rev=1137928&r1=1137927&r2=1137928&view=diff
==============================================================================
--- couchdb/trunk/src/couchdb/couch_replication_manager.erl (original)
+++ couchdb/trunk/src/couchdb/couch_replication_manager.erl Tue Jun 21 10:13:33 2011
@@ -37,9 +37,6 @@
     wait = ?INITIAL_WAIT
 }).
 
--import(couch_replicator_utils, [
-    parse_rep_doc/2
-]).
 -import(couch_util, [
     get_value/2,
     get_value/3,
@@ -120,8 +117,18 @@ init(_) ->
     }}.
 
 
-handle_call({rep_db_update, Change}, _From, State) ->
-    {reply, ok, process_update(State, Change)};
+handle_call({rep_db_update, {ChangeProps} = Change}, _From, State) ->
+    NewState = try
+        process_update(State, Change)
+    catch
+    _Tag:Error ->
+        {RepProps} = get_value(doc, ChangeProps),
+        DocId = get_value(<<"_id">>, RepProps),
+        rep_db_update_error(Error, DocId),
+        State
+    end,
+    {reply, ok, NewState};
+
 
 handle_call({rep_started, RepId}, _From, State) ->
     case rep_state(RepId) of
@@ -309,6 +316,18 @@ process_update(State, {Change}) ->
     end.
 
 
+rep_db_update_error(Error, DocId) ->
+    case Error of
+    {bad_rep_doc, Reason} ->
+        ok;
+    _ ->
+        Reason = to_binary(Error)
+    end,
+    ?LOG_ERROR("Replication manager, error processing document `~s`: ~s",
+        [DocId, Reason]),
+    update_rep_doc(DocId, [{<<"_replication_state">>, <<"error">>}]).
+
+
 rep_user_ctx({RepDoc}) ->
     case get_value(<<"user_ctx">>, RepDoc) of
     undefined ->
@@ -322,8 +341,7 @@ rep_user_ctx({RepDoc}) ->
 
 
 maybe_start_replication(State, DocId, RepDoc) ->
-    {ok, #rep{id = {BaseId, _} = RepId} = Rep} = parse_rep_doc(
-        RepDoc, rep_user_ctx(RepDoc)),
+    #rep{id = {BaseId, _} = RepId} = Rep = parse_rep_doc(RepDoc),
     case rep_state(RepId) of
     nil ->
         RepState = #rep_state{
@@ -354,6 +372,18 @@ maybe_start_replication(State, DocId, Re
     end.
 
 
+parse_rep_doc(RepDoc) ->
+    {ok, Rep} = try
+        couch_replicator_utils:parse_rep_doc(RepDoc, rep_user_ctx(RepDoc))
+    catch
+    throw:{error, Reason} ->
+        throw({bad_rep_doc, Reason});
+    Tag:Err ->
+        throw({bad_rep_doc, to_binary({Tag, Err})})
+    end,
+    Rep.
+
+
 maybe_tag_rep_doc(DocId, {RepProps}, RepId) ->
     case get_value(<<"_replication_id">>, RepProps) of
     RepId ->

Modified: couchdb/trunk/src/couchdb/couch_replicator_utils.erl
URL: http://svn.apache.org/viewvc/couchdb/trunk/src/couchdb/couch_replicator_utils.erl?rev=1137928&r1=1137927&r2=1137928&view=diff
==============================================================================
--- couchdb/trunk/src/couchdb/couch_replicator_utils.erl (original)
+++ couchdb/trunk/src/couchdb/couch_replicator_utils.erl Tue Jun 21 10:13:33 2011
@@ -54,7 +54,17 @@ replication_id(#rep{options = Options} =
 
 replication_id(#rep{user_ctx = UserCtx} = Rep, 2) ->
     {ok, HostName} = inet:gethostname(),
-    Port = mochiweb_socket_server:get(couch_httpd, port),
+    Port = case (catch mochiweb_socket_server:get(couch_httpd, port)) of
+    P when is_number(P) ->
+        P;
+    _ ->
+        % On restart we might be called before the couch_httpd process is
+        % started.
+        % TODO: we might be under an SSL socket server only, or both under
+        % SSL and a non-SSL socket.
+        % ... mochiweb_socket_server:get(https, port)
+        list_to_integer(couch_config:get("httpd", "port", "5984"))
+    end,
     Src = get_rep_endpoint(UserCtx, Rep#rep.source),
     Tgt = get_rep_endpoint(UserCtx, Rep#rep.target),
     maybe_append_filters([HostName, Port, Src, Tgt], Rep);
@@ -85,12 +95,33 @@ maybe_append_filters(Base,
 
 
 filter_code(Filter, Source, UserCtx) ->
-    {match, [DDocName, FilterName]} =
-        re:run(Filter, "(.*?)/(.*)", [{capture, [1, 2], binary}]),
-    {ok, Db} = couch_api_wrap:db_open(Source, [{user_ctx, UserCtx}]),
+    {DDocName, FilterName} =
+    case re:run(Filter, "(.*?)/(.*)", [{capture, [1, 2], binary}]) of
+    {match, [DDocName0, FilterName0]} ->
+        {DDocName0, FilterName0};
+    _ ->
+        throw({error, <<"Invalid filter. Must match `ddocname/filtername`.">>})
+    end,
+    Db = case (catch couch_api_wrap:db_open(Source, [{user_ctx, UserCtx}])) of
+    {ok, Db0} ->
+        Db0;
+    DbError ->
+        DbErrorMsg = io_lib:format("Could not open source database `~s`: ~s",
+           [couch_api_wrap:db_uri(Source), couch_util:to_binary(DbError)]),
+        throw({error, iolist_to_binary(DbErrorMsg)})
+    end,
     try
-        {ok, #doc{body = Body}} = couch_api_wrap:open_doc(
-            Db, <<"_design/", DDocName/binary>>, [ejson_body]),
+        Body = case (catch couch_api_wrap:open_doc(
+            Db, <<"_design/", DDocName/binary>>, [ejson_body])) of
+        {ok, #doc{body = Body0}} ->
+            Body0;
+        DocError ->
+            DocErrorMsg = io_lib:format(
+                "Couldn't open document `_design/~s` from source "
+                "database `~s`: ~s", [couch_api_wrap:db_uri(Source),
+                    DDocName, couch_util:to_binary(DocError)]),
+            throw({error, iolist_to_binary(DocErrorMsg)})
+        end,
         Code = couch_util:get_nested_json_value(
             Body, [<<"filters">>, FilterName]),
         re:replace(Code, [$^, "\s*(.*?)\s*", $$], "\\1", [{return, binary}])