[couchdb] 01/01: A few more cleanups in fabric_doc_open_revs

[va...@apache.org]

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

vatamane pushed a commit to branch dry-fabric-doc-open-revs
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 92c3e33c9cddaff277e71c7f674457976dd24945
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Thu Sep 29 14:25:44 2022 -0400

    A few more cleanups in fabric_doc_open_revs
    
     * Move update_node_revs to a separate function
     * Use a new variable to aviod repeating `ReplyCount + 1` 3 times.
    
    The existing `check_node_rev*` tests should cover the logic and assert that it
    didn't change.
---
 src/fabric/src/fabric_doc_open_revs.erl | 51 ++++++++++++++++++---------------
 1 file changed, 28 insertions(+), 23 deletions(-)

diff --git a/src/fabric/src/fabric_doc_open_revs.erl b/src/fabric/src/fabric_doc_open_revs.erl
index 7909e4c30..bc014385c 100644
--- a/src/fabric/src/fabric_doc_open_revs.erl
+++ b/src/fabric/src/fabric_doc_open_revs.erl
@@ -91,8 +91,10 @@ handle_message({ok, RawReplies}, Worker, State) ->
 
     IsTree = Revs == all orelse Latest,
 
+    NewReplyCount = ReplyCount + 1,
+
     % Do not count error replies when checking quorum
-    RealReplyCount = ReplyCount + 1 - ReplyErrorCount,
+    RealReplyCount = NewReplyCount - ReplyErrorCount,
     QuorumReplies = RealReplyCount >= R,
     {NewReplies, QuorumMet, Repair} =
         case IsTree of
@@ -108,26 +110,8 @@ handle_message({ok, RawReplies}, Worker, State) ->
                 {NewReplies0, MinCount} = dict_replies(PrevReplies, RawReplies),
                 {NewReplies0, MinCount >= R, false}
         end,
-    NewNodeRevs =
-        if
-            Worker == nil ->
-                PrevNodeRevs;
-            true ->
-                IdRevs = lists:foldl(
-                    fun
-                        ({ok, #doc{revs = {Pos, [Rev | _]}}}, Acc) ->
-                            [{Pos, Rev} | Acc];
-                        (_, Acc) ->
-                            Acc
-                    end,
-                    [],
-                    RawReplies
-                ),
-                if
-                    IdRevs == [] -> PrevNodeRevs;
-                    true -> [{Worker#shard.node, IdRevs} | PrevNodeRevs]
-                end
-        end,
+
+    NewNodeRevs = update_node_revs(Worker, RawReplies, PrevNodeRevs),
 
     Complete = (ReplyCount =:= (WorkerCount - 1)),
 
@@ -139,7 +123,7 @@ handle_message({ok, RawReplies}, Worker, State) ->
                 IsTree,
                 NewReplies,
                 NewNodeRevs,
-                ReplyCount + 1,
+                NewReplyCount,
                 InRepair orelse Repair
             ),
             {stop, format_reply(IsTree, NewReplies, RealReplyCount)};
@@ -147,12 +131,33 @@ handle_message({ok, RawReplies}, Worker, State) ->
             {ok, State#state{
                 replies = NewReplies,
                 node_revs = NewNodeRevs,
-                reply_count = ReplyCount + 1,
+                reply_count = NewReplyCount,
                 workers = lists:delete(Worker, Workers),
                 repair = InRepair orelse Repair
             }}
     end.
 
+% Update node revs from the latest replies
+%
+update_node_revs(nil, _, PrevNodeRevs) ->
+    % This handles the error cases when Worker is explicitly set to nil (see
+    % rexi_DOWN and rexi_EXIT clauses
+    PrevNodeRevs;
+update_node_revs(Worker, RawReplies, PrevNodeRevs) ->
+    FoldFun = fun
+        ({ok, #doc{revs = {Pos, [Rev | _]}}}, Acc) ->
+            [{Pos, Rev} | Acc];
+        (_, Acc) ->
+            Acc
+    end,
+    case lists:foldl(FoldFun, [], RawReplies) of
+        [] ->
+            PrevNodeRevs;
+        [_ | _] = IdRevs ->
+            [{Worker#shard.node, IdRevs} | PrevNodeRevs]
+    end.
+
+
 tree_replies(RevTree, []) ->
     {RevTree, true, false};
 tree_replies(RevTree0, [{ok, Doc} | Rest]) ->