(couchdb) 01/01: Prevent a broken mem3 app from permanently failing replication

[va...@apache.org]

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

vatamane pushed a commit to branch prevent-broken-mem3-from-failing-replication-jobs
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 8f52f2db87b6a1fec82c9177dddec0d4ef2c9775
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Nov 7 00:16:10 2024 -0500

    Prevent a broken mem3 app from permanently failing replication
    
    Previously, when mem3 failed such that it's shards ets table disappeared, while
    the replicator was processing doc updates, it was possible for the
    `couch_replicator_utils:owner/2` to raise `error:badarg` (passed through from
    the ets:* calls) to be registered as a malformed (failed) replicator doc
    update. After that, the replication job would be effectively stuck in the
    `failed` state until the user took action to re-create it.
    
    To fix it, change the doc update callback to on only fail jobs on explicit
    replication doc parse failures (they have `throw({the bad_rep_doc, _})
    exception pattern). In any other case let the exception crash the db changes
    feed and let the supervisor restart it.
---
 .../src/couch_replicator_doc_processor.erl         |  5 +--
 .../couch_replicator_scheduler_docs_tests.erl      | 43 ++++++++++++++++++++++
 2 files changed, 44 insertions(+), 4 deletions(-)

diff --git a/src/couch_replicator/src/couch_replicator_doc_processor.erl b/src/couch_replicator/src/couch_replicator_doc_processor.erl
index f22d8b354..57806a158 100644
--- a/src/couch_replicator/src/couch_replicator_doc_processor.erl
+++ b/src/couch_replicator/src/couch_replicator_doc_processor.erl
@@ -95,10 +95,7 @@ db_change(DbName, {ChangeProps} = Change, Server) ->
     try
         ok = process_change(DbName, Change)
     catch
-        exit:{Error, {gen_server, call, [?MODULE, _, _]}} ->
-            ErrMsg = "~p exited ~p while processing change from db ~p",
-            couch_log:error(ErrMsg, [?MODULE, Error, DbName]);
-        _Tag:Error ->
+        throw:{bad_rep_doc, Error} ->
             {RepProps} = get_json_value(doc, ChangeProps),
             DocId = get_json_value(<<"_id">>, RepProps),
             couch_replicator_docs:update_failed(DbName, DocId, Error)
diff --git a/src/couch_replicator/test/eunit/couch_replicator_scheduler_docs_tests.erl b/src/couch_replicator/test/eunit/couch_replicator_scheduler_docs_tests.erl
index acc6abb3a..7cf79da77 100644
--- a/src/couch_replicator/test/eunit/couch_replicator_scheduler_docs_tests.erl
+++ b/src/couch_replicator/test/eunit/couch_replicator_scheduler_docs_tests.erl
@@ -31,6 +31,7 @@ setup_replicator_db(Prefix) ->
     RepDb.
 
 setup_main_replicator_db() ->
+    meck:new(couch_replicator_utils, [passthrough]),
     {Ctx, {Source, Target}} = couch_replicator_test_helper:test_setup(),
     RepDb = setup_replicator_db(<<>>),
     {Ctx, {RepDb, Source, Target}}.
@@ -47,6 +48,7 @@ setup_prefixed_replicator_db_with_update_docs_true() ->
     {Ctx, {RepDb, Source, Target}}.
 
 teardown({Ctx, {RepDb, Source, Target}}) ->
+    meck:unload(),
     ok = fabric:delete_db(RepDb, [?ADMIN_CTX]),
     config:delete("replicator", "update_docs", _Persist = false),
     couch_replicator_test_helper:test_teardown({Ctx, {Source, Target}}).
@@ -117,6 +119,16 @@ t_replicator_doc_state_fields_update_docs_true_test_() ->
         ])
     }.
 
+t_replicator_doc_doesnt_fail_when_mem3_breaks_test_() ->
+    {
+        setup,
+        fun setup_main_replicator_db/0,
+        fun teardown/1,
+        with([
+            ?TDEF(t_doc_doesnt_fail_when_mem3_breaks, 30)
+        ])
+    }.
+
 t_scheduler_docs_total_rows({_Ctx, {RepDb, Source, Target}}) ->
     SourceUrl = couch_replicator_test_helper:cluster_db_url(Source),
     TargetUrl = couch_replicator_test_helper:cluster_db_url(Target),
@@ -239,6 +251,37 @@ t_doc_fields_are_ignored({_Ctx, {RepDb, Source, Target}}) ->
         StateDoc
     ).
 
+t_doc_doesnt_fail_when_mem3_breaks({_Ctx, {RepDb, Source, Target}}) ->
+    SourceUrl = couch_replicator_test_helper:cluster_db_url(Source),
+    TargetUrl = couch_replicator_test_helper:cluster_db_url(Target),
+    RepDoc = #{
+        <<"source">> => SourceUrl,
+        <<"target">> => TargetUrl
+    },
+    % Make owner function fail once
+    meck:expect(
+        couch_replicator_utils,
+        owner,
+        2,
+        meck:seq([
+            meck:raise(error, badarg),
+            meck:passthrough()
+        ])
+    ),
+    RepDocUrl = rep_doc_url(RepDb, ?docid()),
+    {201, _} = req(put, RepDocUrl, RepDoc),
+    StateDoc = test_util:wait(
+        fun() ->
+            case req(get, RepDocUrl) of
+                {200, #{<<"_replication_state">> := <<"completed">>} = StDoc} -> StDoc;
+                {_, #{}} -> wait
+            end
+        end,
+        14000,
+        1000
+    ),
+    ?assertMatch(#{<<"_replication_state">> := <<"completed">>}, StateDoc).
+
 rep_doc_url(RepDb, DocId) when is_binary(RepDb) ->
     rep_doc_url(binary_to_list(RepDb), DocId);
 rep_doc_url(RepDb, DocId) when is_binary(DocId) ->