chewbranca commented on code in PR #4862: URL: https://github.com/apache/couchdb/pull/4862#discussion_r1412569187
##########
src/fabric/src/fabric_rpc.erl:
##########
@@ -535,6 +535,32 @@ changes_enumerator(#full_doc_info{} = FDI, Acc) ->
changes_enumerator(#doc_info{id = <<"_local/", _/binary>>, high_seq = Seq},
Acc) ->
{ok, Acc#fabric_changes_acc{seq = Seq, pending =
Acc#fabric_changes_acc.pending - 1}};
changes_enumerator(DocInfo, Acc) ->
+ #fabric_changes_acc{
+ db = Db,
+ args = #changes_args{
+ include_docs = IncludeDocs,
+ filter_fun = Filter
+ },
+ pending = Pending
+ } = Acc,
+ #doc_info{high_seq = Seq} = DocInfo,
+ RevsOrDocRevs = couch_changes:filter(Db, DocInfo, Filter, IncludeDocs),
+ % include_docs = false, call `filter/3`, use `couch_changes:open_revs/3`
to read docs.
+ % include_docs = true, call `filter/4`, it will return [revs] or {docs,
revs}.
+ % - {}: use `couch_changes:open_revs/3` to open docs
+ % - []: use `fabric_rpc:doc_member/3` to open docs
+ ChangesRow =
+ case is_tuple(RevsOrDocRevs) of
+ true ->
+ {Docs, Revs} = RevsOrDocRevs,
+ get_changes_row(Revs, Acc, DocInfo, fun get_json_docs/3, Docs);
+ false ->
+ get_changes_row(RevsOrDocRevs, Acc, DocInfo, fun doc_member/3,
{Db, DocInfo})
+ end,
Review Comment:
Teehee got nerd sniped a bit on reworking `get_changes_row`. I think we can
isolate the loading of the docs from the generate of the row itself, and
simplify things a bit. I haven't tested this or anything wild like that, but
how about something like:
```diff
diff --git a/src/fabric/src/fabric_rpc.erl b/src/fabric/src/fabric_rpc.erl
index 42544ac40..1e611e131 100644
--- a/src/fabric/src/fabric_rpc.erl
+++ b/src/fabric/src/fabric_rpc.erl
@@ -539,36 +539,40 @@ changes_enumerator(DocInfo, Acc) ->
db = Db,
args = #changes_args{
include_docs = IncludeDocs,
- filter_fun = Filter
+ conflicts = Conflicts,
+ filter_fun = Filter,
+ doc_options = DocOptions
},
pending = Pending
} = Acc,
+ Opts =
+ if
+ Conflicts -> [conflicts | DocOptions];
+ true -> DocOptions
+ end,
#doc_info{high_seq = Seq} = DocInfo,
- RevsOrDocRevs = couch_changes:filter(Db, DocInfo, Filter, IncludeDocs),
- % include_docs = false, call `filter/3`, use
`couch_changes:open_revs/3` to read docs.
- % include_docs = true, call `filter/4`, it will return [revs] or {docs,
revs}.
- % - {}: use `couch_changes:open_revs/3` to open docs
- % - []: use `fabric_rpc:doc_member/3` to open docs
+ {Docs, Revs} = couch_changes:filter(Db, DocInfo, Filter, IncludeDocs),
ChangesRow =
- case is_tuple(RevsOrDocRevs) of
- true ->
- {Docs, Revs} = RevsOrDocRevs,
- get_changes_row(Revs, Acc, DocInfo, fun get_json_docs/3,
Docs);
- false ->
- get_changes_row(RevsOrDocRevs, Acc, DocInfo, fun
doc_member/3, {Db, DocInfo})
+ case {Docs, IncludeDocs} of
+ %% TODO: is it possible to hit `{[], [_|_], true} = {Revs,
Docs, IncludeDocs}`?
+ %% if so, we'll load the docs when we should `no_pass` below
+ {[], true} ->
+ Docs1 = [doc_member({Db, DocInfo}, Opts, Filter)],
+ get_changes_row(Revs, Acc, DocInfo, Docs1);
+ {[], false} ->
+ get_changes_row(Revs, Acc, DocInfo, []);
+ {Docs, true} ->
+ Docs1 = [get_json_docs(Docs, Opts, Filter)],
+ get_changes_row(Revs, Acc, DocInfo, Docs1);
+ {_Docs, false} ->
+ get_changes_row(Revs, Acc, DocInfo, [])
end,
ok = rexi:stream2(ChangesRow),
{ok, Acc#fabric_changes_acc{seq = Seq, pending = Pending - 1}}.
-get_changes_row(Revs, Acc, DocInfo, Fun, Args) ->
+get_changes_row(Revs, Acc, DocInfo, Docs) ->
#fabric_changes_acc{
db = Db,
- args = #changes_args{
- include_docs = IncludeDocs,
- conflicts = Conflicts,
- filter_fun = Filter,
- doc_options = DocOptions
- },
pending = Pending,
epochs = Epochs
} = Acc,
@@ -580,21 +584,13 @@ get_changes_row(Revs, Acc, DocInfo, Fun, Args) ->
{seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}}
]};
Results ->
- Opts =
- if
- Conflicts -> [conflicts | DocOptions];
- true -> DocOptions
- end,
{change, [
{pending, Pending - 1},
{seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}},
{id, Id},
{changes, Results},
{deleted, Del}
- | if
- IncludeDocs -> [Fun(Args, Opts, Filter)];
- true -> []
- end
+ | Docs
]}
end.
```
and that's kind of a big delta, so here's what it looks like afterwards:
```erlang
changes_enumerator(#full_doc_info{} = FDI, Acc) ->
changes_enumerator(couch_doc:to_doc_info(FDI), Acc);
changes_enumerator(#doc_info{id = <<"_local/", _/binary>>, high_seq = Seq},
Acc) ->
{ok, Acc#fabric_changes_acc{seq = Seq, pending =
Acc#fabric_changes_acc.pending - 1}};
changes_enumerator(DocInfo, Acc) ->
#fabric_changes_acc{
db = Db,
args = #changes_args{
include_docs = IncludeDocs,
conflicts = Conflicts,
filter_fun = Filter,
doc_options = DocOptions
},
pending = Pending
} = Acc,
Opts =
if
Conflicts -> [conflicts | DocOptions];
true -> DocOptions
end,
#doc_info{high_seq = Seq} = DocInfo,
{Docs, Revs} = couch_changes:filter(Db, DocInfo, Filter, IncludeDocs),
ChangesRow =
case {Docs, IncludeDocs} of
%% TODO: is it possible to hit `{[], [_|_], true} = {Revs, Docs,
IncludeDocs}`?
%% if so, we'll load the docs when we should `no_pass` below
{[], true} ->
Docs1 = [doc_member({Db, DocInfo}, Opts, Filter)],
get_changes_row(Revs, Acc, DocInfo, Docs1);
{[], false} ->
get_changes_row(Revs, Acc, DocInfo, []);
{Docs, true} ->
Docs1 = [get_json_docs(Docs, Opts, Filter)],
get_changes_row(Revs, Acc, DocInfo, Docs1);
{_Docs, false} ->
get_changes_row(Revs, Acc, DocInfo, [])
end,
ok = rexi:stream2(ChangesRow),
{ok, Acc#fabric_changes_acc{seq = Seq, pending = Pending - 1}}.
get_changes_row(Revs, Acc, DocInfo, Docs) ->
#fabric_changes_acc{
db = Db,
pending = Pending,
epochs = Epochs
} = Acc,
#doc_info{id = Id, high_seq = Seq, revs = [#rev_info{deleted = Del} |
_]} = DocInfo,
case [X || X <- Revs, X /= null] of
[] ->
{no_pass, [
{pending, Pending - 1},
{seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}}
]};
Results ->
{change, [
{pending, Pending - 1},
{seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}},
{id, Id},
{changes, Results},
{deleted, Del}
| Docs
]}
end.
```
Maybe something like that could work?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
