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 2019/07/12 14:40:23 UTC
[couchdb] 01/03: Fix bulk docs error reporting
This is an automated email from the ASF dual-hosted git repository.
davisp pushed a commit to branch prototype/fdb-layer
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 8d167afaf3592270282e801ea4e0431c78c7dc3e
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Thu Jul 11 14:25:24 2019 -0500
Fix bulk docs error reporting
The existing logic around return codes and term formats is labyrinthine.
This is the result of much trial and error to get the new logic to
behave exactly the same as the previous implementation.
---
src/chttpd/src/chttpd_db.erl | 2 +
src/fabric/src/fabric2_db.erl | 108 +++++++++++++++++------------
src/fabric/test/fabric2_doc_crud_tests.erl | 20 ++----
3 files changed, 73 insertions(+), 57 deletions(-)
diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index 90869c6..abdd825 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -1337,6 +1337,8 @@ update_doc_result_to_json(DocId, {ok, NewRev}) ->
{[{ok, true}, {id, DocId}, {rev, couch_doc:rev_to_str(NewRev)}]};
update_doc_result_to_json(DocId, {accepted, NewRev}) ->
{[{ok, true}, {id, DocId}, {rev, couch_doc:rev_to_str(NewRev)}, {accepted, true}]};
+update_doc_result_to_json(DocId, {{DocId, _}, Error}) ->
+ update_doc_result_to_json(DocId, Error);
update_doc_result_to_json(DocId, Error) ->
{_Code, ErrorStr, Reason} = chttpd:error_info(Error),
{[{id, DocId}, {error, ErrorStr}, {reason, Reason}]}.
diff --git a/src/fabric/src/fabric2_db.erl b/src/fabric/src/fabric2_db.erl
index eb74a18..3ea30e7 100644
--- a/src/fabric/src/fabric2_db.erl
+++ b/src/fabric/src/fabric2_db.erl
@@ -584,46 +584,52 @@ update_docs(Db, Docs) ->
update_docs(Db, Docs0, Options) ->
Docs1 = apply_before_doc_update(Db, Docs0, Options),
- Resps0 = case lists:member(replicated_changes, Options) of
- false ->
- fabric2_fdb:transactional(Db, fun(TxDb) ->
- update_docs_interactive(TxDb, Docs1, Options)
- end);
- true ->
- lists:map(fun(Doc) ->
+ try
+ validate_atomic_update(Docs0, lists:member(all_or_nothing, Options)),
+ Resps0 = case lists:member(replicated_changes, Options) of
+ false ->
fabric2_fdb:transactional(Db, fun(TxDb) ->
- update_doc_int(TxDb, Doc, Options)
- end)
- end, Docs1)
- end,
- % Convert errors
- Resps1 = lists:map(fun(Resp) ->
- case Resp of
- {#doc{} = Doc, Error} ->
- #doc{
- id = DocId,
- revs = Revs
- } = Doc,
- RevId = case Revs of
- {RevPos, [Rev | _]} -> {RevPos, Rev};
- {0, []} -> {0, <<>>}
- end,
- {{DocId, RevId}, Error};
- Else ->
- Else
+ update_docs_interactive(TxDb, Docs1, Options)
+ end);
+ true ->
+ lists:map(fun(Doc) ->
+ fabric2_fdb:transactional(Db, fun(TxDb) ->
+ update_doc_int(TxDb, Doc, Options)
+ end)
+ end, Docs1)
+ end,
+ % Convert errors
+ Resps1 = lists:map(fun(Resp) ->
+ case Resp of
+ {#doc{} = Doc, Error} ->
+ #doc{
+ id = DocId,
+ revs = Revs
+ } = Doc,
+ RevId = case Revs of
+ {RevPos, [Rev | _]} -> {RevPos, Rev};
+ {0, []} -> {0, <<>>};
+ Else -> Else
+ end,
+ {{DocId, RevId}, Error};
+ Else ->
+ Else
+ end
+ end, Resps0),
+ case lists:member(replicated_changes, Options) of
+ true ->
+ {ok, lists:flatmap(fun(R) ->
+ case R of
+ {ok, []} -> [];
+ {{_, _}, {ok, []}} -> [];
+ Else -> [Else]
+ end
+ end, Resps1)};
+ false ->
+ {ok, Resps1}
end
- end, Resps0),
- case lists:member(replicated_changes, Options) of
- true ->
- {ok, [R || R <- Resps1, R /= {ok, []}]};
- false ->
- Status = lists:foldl(fun(Resp, Acc) ->
- case Resp of
- {ok, _} -> Acc;
- _ -> error
- end
- end, ok, Resps1),
- {Status, Resps1}
+ catch throw:{aborted, Errors} ->
+ {aborted, Errors}
end.
@@ -1023,7 +1029,7 @@ update_docs_interactive(Db, #doc{id = <<?LOCAL_DOC_PREFIX, _/binary>>} = Doc,
update_docs_interactive(Db, Doc, Options, Futures, SeenIds) ->
case lists:member(Doc#doc.id, SeenIds) of
true ->
- {{error, conflict}, SeenIds};
+ {conflict, SeenIds};
false ->
Future = maps:get(doc_tag(Doc), Futures),
case update_doc_interactive(Db, Doc, Future, Options) of
@@ -1066,12 +1072,12 @@ update_doc_interactive(Db, Doc0, Future, _Options) ->
% Check that a revision was specified if required
Doc0RevId = doc_to_revid(Doc0),
if Doc0RevId /= {0, <<>>} orelse WinnerRevId == {0, <<>>} -> ok; true ->
- ?RETURN({error, conflict})
+ ?RETURN({Doc0, conflict})
end,
% Check that we're not trying to create a deleted doc
if Doc0RevId /= {0, <<>>} orelse not Doc0#doc.deleted -> ok; true ->
- ?RETURN({error, conflict})
+ ?RETURN({Doc0, conflict})
end,
% Get the target revision to update
@@ -1088,7 +1094,7 @@ update_doc_interactive(Db, Doc0, Future, _Options) ->
% that we get not_found for a deleted revision
% because we only check for the non-deleted
% key in fdb
- ?RETURN({error, conflict})
+ ?RETURN({Doc0, conflict})
end
end,
@@ -1191,7 +1197,7 @@ update_doc_replicated(Db, Doc0, _Options) ->
if Status /= internal_node -> ok; true ->
% We already know this revision so nothing
% left to do.
- ?RETURN({ok, []})
+ ?RETURN({Doc0, {ok, []}})
end,
% Its possible to have a replication with fewer than $revs_limit
@@ -1248,7 +1254,7 @@ update_doc_replicated(Db, Doc0, _Options) ->
update_local_doc(Db, Doc0, _Options) ->
Doc1 = case increment_local_doc_rev(Doc0) of
{ok, Updated} -> Updated;
- {error, _} = Error -> ?RETURN(Error)
+ {error, Error} -> ?RETURN({Doc0, Error})
end,
ok = fabric2_fdb:write_local_doc(Db, Doc1),
@@ -1367,6 +1373,20 @@ validate_ddoc(Db, DDoc) ->
end.
+validate_atomic_update(_, false) ->
+ ok;
+validate_atomic_update(AllDocs, true) ->
+ % TODO actually perform the validation. This requires some hackery, we need
+ % to basically extract the prep_and_validate_updates function from couch_db
+ % and only run that, without actually writing in case of a success.
+ Error = {not_implemented, <<"all_or_nothing is not supported">>},
+ PreCommitFailures = lists:map(fun(#doc{id=Id, revs = {Pos,Revs}}) ->
+ case Revs of [] -> RevId = <<>>; [RevId|_] -> ok end,
+ {{Id, {Pos, RevId}}, Error}
+ end, AllDocs),
+ throw({aborted, PreCommitFailures}).
+
+
check_duplicate_attachments(#doc{atts = Atts}) ->
lists:foldl(fun(Att, Names) ->
Name = couch_att:fetch(name, Att),
diff --git a/src/fabric/test/fabric2_doc_crud_tests.erl b/src/fabric/test/fabric2_doc_crud_tests.erl
index 85b2766..c19c474 100644
--- a/src/fabric/test/fabric2_doc_crud_tests.erl
+++ b/src/fabric/test/fabric2_doc_crud_tests.erl
@@ -408,7 +408,7 @@ conflict_on_create_new_with_rev({Db, _}) ->
revs = {1, [fabric2_util:uuid()]},
body = {[{<<"foo">>, <<"bar">>}]}
},
- ?assertThrow({error, conflict}, fabric2_db:update_doc(Db, Doc)).
+ ?assertThrow(conflict, fabric2_db:update_doc(Db, Doc)).
conflict_on_update_with_no_rev({Db, _}) ->
@@ -421,7 +421,7 @@ conflict_on_update_with_no_rev({Db, _}) ->
revs = {0, []},
body = {[{<<"state">>, 2}]}
},
- ?assertThrow({error, conflict}, fabric2_db:update_doc(Db, Doc2)).
+ ?assertThrow(conflict, fabric2_db:update_doc(Db, Doc2)).
conflict_on_create_as_deleted({Db, _}) ->
@@ -430,7 +430,7 @@ conflict_on_create_as_deleted({Db, _}) ->
deleted = true,
body = {[{<<"foo">>, <<"bar">>}]}
},
- ?assertThrow({error, conflict}, fabric2_db:update_doc(Db, Doc)).
+ ?assertThrow(conflict, fabric2_db:update_doc(Db, Doc)).
conflict_on_recreate_as_deleted({Db, _}) ->
@@ -450,7 +450,7 @@ conflict_on_recreate_as_deleted({Db, _}) ->
deleted = true,
body = {[{<<"state">>, 3}]}
},
- ?assertThrow({error, conflict}, fabric2_db:update_doc(Db, Doc3)).
+ ?assertThrow(conflict, fabric2_db:update_doc(Db, Doc3)).
conflict_on_extend_deleted({Db, _}) ->
@@ -470,7 +470,7 @@ conflict_on_extend_deleted({Db, _}) ->
deleted = false,
body = {[{<<"state">>, 3}]}
},
- ?assertThrow({error, conflict}, fabric2_db:update_doc(Db, Doc3)).
+ ?assertThrow(conflict, fabric2_db:update_doc(Db, Doc3)).
open_doc_revs_basic({Db, _}) ->
@@ -725,18 +725,12 @@ create_local_doc_bad_rev({Db, _}) ->
id = LDocId,
revs = {0, [<<"not a number">>]}
},
- ?assertThrow(
- {error, <<"Invalid rev format">>},
- fabric2_db:update_doc(Db, Doc1)
- ),
+ ?assertThrow(<<"Invalid rev format">>, fabric2_db:update_doc(Db, Doc1)),
Doc2 = Doc1#doc{
revs = bad_bad_rev_roy_brown
},
- ?assertThrow(
- {error, <<"Invalid rev format">>},
- fabric2_db:update_doc(Db, Doc2)
- ).
+ ?assertThrow(<<"Invalid rev format">>, fabric2_db:update_doc(Db, Doc2)).
create_local_doc_random_rev({Db, _}) ->