You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2023/09/18 19:17:16 UTC
[couchdb] branch replace-kaboom updated (2427fbda1 -> 49580133e)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch replace-kaboom
in repository https://gitbox.apache.org/repos/asf/couchdb.git
omit 2427fbda1 Replace replicator open_doc_revs kaboom with something more descriptive
new 49580133e Replace replicator open_doc_revs kaboom with something more descriptive
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 (2427fbda1)
\
N -- N -- N refs/heads/replace-kaboom (49580133e)
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 1 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:
.../src/couch_replicator_api_wrap.erl | 33 +++++-----
.../couch_replicator_error_reporting_tests.erl | 72 +++++++++++++++-------
2 files changed, 65 insertions(+), 40 deletions(-)
[couchdb] 01/01: Replace replicator open_doc_revs kaboom with something more descriptive
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch replace-kaboom
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 49580133ec7ac1a981023c62675303ff583ffa28
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Mon Sep 18 11:52:17 2023 -0400
Replace replicator open_doc_revs kaboom with something more descriptive
Use something more descriptive, especially since it can bubble up the user level
when the replication job crashes.
In order to also log the error causing the failure, move the exit call to the
main open_doc_revs body and the remove `retries=0` clause. However, avoid
changing the exit value shape and keep it as an atom for now. There is a chance
it might break error handling in the upper API layers in case when we match on
error reason "shape".
To test everything end-to-end, modify the test setup to create replication jobs
with additional options, when previously it was only a choice between
continuous or one-shot.
---
.../src/couch_replicator_api_wrap.erl | 33 +++++-----
.../couch_replicator_error_reporting_tests.erl | 72 +++++++++++++++-------
2 files changed, 65 insertions(+), 40 deletions(-)
diff --git a/src/couch_replicator/src/couch_replicator_api_wrap.erl b/src/couch_replicator/src/couch_replicator_api_wrap.erl
index fbe9538ad..3dac826a0 100644
--- a/src/couch_replicator/src/couch_replicator_api_wrap.erl
+++ b/src/couch_replicator/src/couch_replicator_api_wrap.erl
@@ -277,14 +277,6 @@ bulk_get_zip({Id, Rev, _}, {[_ | _] = Props}) ->
end.
-spec open_doc_revs(#httpdb{}, binary(), list(), list(), function(), any()) -> no_return().
-open_doc_revs(#httpdb{retries = 0} = HttpDb, Id, Revs, Options, _Fun, _Acc) ->
- Path = encode_doc_id(Id),
- QS = options_to_query_args(HttpDb, Path, [revs, {open_revs, Revs} | Options]),
- Url = couch_util:url_strip_password(
- couch_replicator_httpc:full_url(HttpDb, [{path, Path}, {qs, QS}])
- ),
- couch_log:error("Replication crashing because GET ~s failed", [Url]),
- exit(kaboom);
open_doc_revs(#httpdb{} = HttpDb, Id, Revs, Options, Fun, Acc) ->
Path = encode_doc_id(Id),
QS = options_to_query_args(HttpDb, Path, [revs, {open_revs, Revs} | Options]),
@@ -369,17 +361,20 @@ open_doc_revs(#httpdb{} = HttpDb, Id, Revs, Options, Fun, Acc) ->
couch_replicator_httpc:full_url(HttpDb, [{path, Path}, {qs, QS}])
),
#httpdb{retries = Retries, wait = Wait0} = HttpDb,
- Wait = 2 * erlang:min(Wait0 * 2, ?MAX_WAIT),
- couch_log:notice(
- "Retrying GET to ~s in ~p seconds due to error ~w",
- [Url, Wait / 1000, error_reason(Else)]
- ),
- ok = timer:sleep(Wait),
- RetryDb = HttpDb#httpdb{
- retries = Retries - 1,
- wait = Wait
- },
- open_doc_revs(RetryDb, Id, Revs, Options, Fun, Acc)
+ NewRetries = Retries - 1,
+ case NewRetries > 0 of
+ true ->
+ Wait = 2 * erlang:min(Wait0 * 2, ?MAX_WAIT),
+ LogRetryMsg = "Retrying GET to ~s in ~p seconds due to error ~w",
+ couch_log:notice(LogRetryMsg, [Url, Wait / 1000, error_reason(Else)]),
+ ok = timer:sleep(Wait),
+ RetryDb = HttpDb#httpdb{retries = NewRetries, wait = Wait},
+ open_doc_revs(RetryDb, Id, Revs, Options, Fun, Acc);
+ false ->
+ LogFailMsg = "Replication crashing because GET ~s failed. Error ~p",
+ couch_log:error(LogFailMsg, [Url, error_reason(Else)]),
+ exit(open_doc_revs_failed)
+ end
end.
error_reason({http_request_failed, "GET", _Url, {error, timeout}}) ->
diff --git a/src/couch_replicator/test/eunit/couch_replicator_error_reporting_tests.erl b/src/couch_replicator/test/eunit/couch_replicator_error_reporting_tests.erl
index 30bc12c29..2441f0cd7 100644
--- a/src/couch_replicator/test/eunit/couch_replicator_error_reporting_tests.erl
+++ b/src/couch_replicator/test/eunit/couch_replicator_error_reporting_tests.erl
@@ -34,6 +34,7 @@ error_reporting_test_() ->
?TDEF_FE(t_skip_doc_put_invalid_attachment_name),
?TDEF_FE(t_fail_revs_diff),
?TDEF_FE(t_fail_bulk_get, 15),
+ ?TDEF_FE(t_fail_open_docs_get, 15),
?TDEF_FE(t_fail_changes_queue),
?TDEF_FE(t_fail_changes_manager),
?TDEF_FE(t_fail_changes_reader_proc),
@@ -120,7 +121,7 @@ t_skip_doc_put_401_errors({_Ctx, {Source, Target}}) ->
populate_db(Source, 6, 6, _WithAttachments = true),
ErrBody = [<<"{\"error\":\"unauthorized\", \"reason\":\"vdu\"}">>],
mock_fail_req(put, "/6", {ok, "401", [], ErrBody}),
- {ok, RepId} = replicate(Source, Target, false),
+ {ok, RepId} = replicate(Source, Target, #{continuous => false}),
{ok, Listener} = rep_result_listener(RepId),
Res = wait_rep_result(RepId),
% Replication job should succeed
@@ -140,7 +141,7 @@ t_skip_doc_put_403_errors({_Ctx, {Source, Target}}) ->
populate_db(Source, 6, 6, _WithAttachments = true),
ErrBody = [<<"{\"error\":\"forbidden\", \"reason\":\"vdu\"}">>],
mock_fail_req(put, "/6", {ok, "403", [], ErrBody}),
- {ok, RepId} = replicate(Source, Target, false),
+ {ok, RepId} = replicate(Source, Target, #{continuous => false}),
{ok, Listener} = rep_result_listener(RepId),
Res = wait_rep_result(RepId),
% Replication job should succeed
@@ -160,7 +161,7 @@ t_skip_doc_put_413_errors({_Ctx, {Source, Target}}) ->
populate_db(Source, 6, 6, _WithAttachments = true),
ErrBody = [<<"{\"error\":\"too_large\", \"reason\":\"too_large\"}">>],
mock_fail_req(put, "/6", {ok, "413", [], ErrBody}),
- {ok, RepId} = replicate(Source, Target, false),
+ {ok, RepId} = replicate(Source, Target, #{continuous => false}),
{ok, Listener} = rep_result_listener(RepId),
Res = wait_rep_result(RepId),
% Replication job should succeed
@@ -180,7 +181,7 @@ t_skip_doc_put_415_errors({_Ctx, {Source, Target}}) ->
populate_db(Source, 6, 6, _WithAttachments = true),
ErrBody = [<<"{\"error\":\"unsupported_media_type\", \"reason\":\"bad_media\"}">>],
mock_fail_req(put, "/6", {ok, "415", [], ErrBody}),
- {ok, RepId} = replicate(Source, Target, false),
+ {ok, RepId} = replicate(Source, Target, #{continuous => false}),
{ok, Listener} = rep_result_listener(RepId),
Res = wait_rep_result(RepId),
% Replication job should succeed
@@ -202,7 +203,7 @@ t_skip_doc_put_invalid_attachment_name({_Ctx, {Source, Target}}) ->
<<"{\"error\":\"bad_request\", \"reason\":\"Attachment name '_foo' starts with prohibited character '_'\"}">>
],
mock_fail_req(put, "/6", {ok, "400", [], ErrBody}),
- {ok, RepId} = replicate(Source, Target, false),
+ {ok, RepId} = replicate(Source, Target, #{continuous => false}),
{ok, Listener} = rep_result_listener(RepId),
Res = wait_rep_result(RepId),
% Replication job should succeed
@@ -256,6 +257,33 @@ t_fail_bulk_get({_Ctx, {Source, Target}}) ->
% Check that there was a falback to a plain GET
?assertEqual(1, meck:num_calls(couch_replicator_api_wrap, open_doc_revs, 6)).
+t_fail_open_docs_get({_Ctx, {Source, Target}}) ->
+ populate_db(Source, 1, 5),
+ Opts = #{
+ % We're testing the case of individual doc rev GETs
+ use_bulk_get => false,
+ % Perform at least one retry before giving up (for extra coverage)
+ retries_per_request => 2
+ },
+ {ok, RepId} = replicate(Source, Target, Opts),
+ wait_target_in_sync(Source, Target),
+
+ {ok, Listener} = rep_result_listener(RepId),
+ % Break open_doc_revs on the server side and see what happens
+ meck:new(fabric_doc_open_revs, [passthrough]),
+ meck:expect(fabric_doc_open_revs, go, fun
+ (Source, <<"6">>, _, _) ->
+ % This is a random error, no particular reason for a 404
+ meck:exception(throw, not_found);
+ (ArgDb, ArgDocId, ArgRevs, ArgOpts) ->
+ meck:passthrough([ArgDb, ArgDocId, ArgRevs, ArgOpts])
+ end),
+ populate_db(Source, 6, 6),
+ {error, Result} = wait_rep_result(RepId),
+ ?assertMatch({worker_died, _, {process_died, _, open_doc_revs_failed}}, Result),
+ ?assert(meck:num_calls(fabric_doc_open_revs, go, 4) >= 2),
+ couch_replicator_notifier:stop(Listener).
+
t_fail_changes_queue({_Ctx, {Source, Target}}) ->
populate_db(Source, 1, 5),
{ok, RepId} = replicate(Source, Target),
@@ -311,7 +339,7 @@ t_dont_start_duplicate_job({_Ctx, {Source, Target}}) ->
meck:new(couch_replicator_pg, [passthrough]),
Pid = pid_from_another_node(),
meck:expect(couch_replicator_pg, should_start, fun(_, _) -> {no, Pid} end),
- Rep = make_rep(Source, Target, true),
+ Rep = make_rep(Source, Target, #{continuous => true}),
ExpectErr = {error, {already_started, Pid}},
?assertEqual(ExpectErr, couch_replicator_scheduler_job:start_link(Rep)).
@@ -444,26 +472,28 @@ wait_target_in_sync_loop(DocCount, TargetName, RetriesLeft) ->
end.
replicate(Source, Target) ->
- replicate(Source, Target, true).
+ replicate(Source, Target, #{}).
-replicate(Source, Target, Continuous) ->
- Rep = make_rep(Source, Target, Continuous),
+replicate(Source, Target, #{} = Opts) ->
+ Rep = make_rep(Source, Target, Opts),
ok = couch_replicator_scheduler:add_job(Rep),
couch_replicator_scheduler:reschedule(),
{ok, Rep#rep.id}.
-make_rep(Source, Target, Continuous) ->
- RepObject =
- {[
- {<<"source">>, url(Source)},
- {<<"target">>, url(Target)},
- {<<"continuous">>, Continuous},
- {<<"worker_processes">>, 1},
- {<<"retries_per_request">>, 1},
- % Low connection timeout so _changes feed gets restarted quicker
- {<<"connection_timeout">>, 3000}
- ]},
- {ok, Rep} = couch_replicator_parse:parse_rep_doc(RepObject, ?ADMIN_USER),
+make_rep(Source, Target, #{} = OverrideOpts) ->
+ Opts0 = #{
+ source => url(Source),
+ target => url(Target),
+ continuous => true,
+ worker_processes => 1,
+ retries_per_request => 1,
+ % Low connection timeout so _changes feed gets restarted quicker
+ connection_timeout => 3000
+ },
+ RepMap = maps:merge(Opts0, OverrideOpts),
+ % parse_rep_doc accepts {[...]} ejson format
+ RepEJson = couch_util:json_decode(couch_util:json_encode(RepMap)),
+ {ok, Rep} = couch_replicator_parse:parse_rep_doc(RepEJson, ?ADMIN_USER),
Rep.
url(DbName) ->