You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2022/09/29 18:29:30 UTC
[GitHub] [couchdb] nickva opened a new pull request, #4193: A few more cleanups in fabric_doc_open_revs
nickva opened a new pull request, #4193:
URL: https://github.com/apache/couchdb/pull/4193
* Move update_node_revs to a separate function
* Use a new variable to avoid repeating `ReplyCount + 1` 3 times.
The existing `check_node_rev*` tests should cover the logic and assert that it didn't change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] jaydoane commented on a diff in pull request #4193: A few more cleanups in fabric_doc_open_revs
Posted by GitBox <gi...@apache.org>.
jaydoane commented on code in PR #4193:
URL: https://github.com/apache/couchdb/pull/4193#discussion_r983942863
##########
src/fabric/src/fabric_doc_open_revs.erl:
##########
@@ -139,20 +123,40 @@ handle_message({ok, RawReplies}, Worker, State) ->
IsTree,
NewReplies,
NewNodeRevs,
- ReplyCount + 1,
+ NewReplyCount,
InRepair orelse Repair
),
{stop, format_reply(IsTree, NewReplies, RealReplyCount)};
false ->
{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
Review Comment:
append ")." to close parenthetical statement
##########
src/fabric/src/fabric_doc_open_revs.erl:
##########
@@ -139,20 +123,40 @@ handle_message({ok, RawReplies}, Worker, State) ->
IsTree,
NewReplies,
NewNodeRevs,
- ReplyCount + 1,
+ NewReplyCount,
InRepair orelse Repair
),
{stop, format_reply(IsTree, NewReplies, RealReplyCount)};
false ->
{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.
Review Comment:
This is a great readability improvement over the previous logic 👏
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva commented on a diff in pull request #4193: A few more cleanups in fabric_doc_open_revs
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4193:
URL: https://github.com/apache/couchdb/pull/4193#discussion_r983999143
##########
src/fabric/src/fabric_doc_open_revs.erl:
##########
@@ -139,20 +123,40 @@ handle_message({ok, RawReplies}, Worker, State) ->
IsTree,
NewReplies,
NewNodeRevs,
- ReplyCount + 1,
+ NewReplyCount,
InRepair orelse Repair
),
{stop, format_reply(IsTree, NewReplies, RealReplyCount)};
false ->
{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
Review Comment:
Oh good catch!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb] nickva merged pull request #4193: A few more cleanups in fabric_doc_open_revs
Posted by GitBox <gi...@apache.org>.
nickva merged PR #4193:
URL: https://github.com/apache/couchdb/pull/4193
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@couchdb.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org