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

jiahuili430 pushed a commit to branch fix-changes-stats
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit a7ecd8f88faddd85b96f3e1bfd8299ce495ceb86
Author: Jiahui Li <[email protected]>
AuthorDate: Tue Dec 5 21:21:52 2023 -0600

    Avoid read docs twice when filtered _changes is triggered
    
    - When filered `_changes` is triggered, `open_revs()` will
    read docs. If request parameter has `include_docs=true`,
    then `doc_member()` will also read docs.
    
    Change `filter/3` to `filter/5` to avoid the above behavior.
    
    - When using filtered `_changes` with `style=all_docs`,
    `conflicts=true` and `include_docs=true`, the response should
    contain the `_conflicts` field.
    
    Add `open_all_revs_include_doc/2` to include this field.
    
    - Add tests to verify the number of calls to `open_doc/3`.
---
 src/couch/src/couch_changes.erl | 98 +++++++++++++++++++++++++++++------------
 src/fabric/src/fabric_rpc.erl   | 61 +++++++++++++++----------
 2 files changed, 106 insertions(+), 53 deletions(-)

diff --git a/src/couch/src/couch_changes.erl b/src/couch/src/couch_changes.erl
index e072a2e1c..59a1883c8 100644
--- a/src/couch/src/couch_changes.erl
+++ b/src/couch/src/couch_changes.erl
@@ -19,7 +19,7 @@
     wait_updated/3,
     get_rest_updated/1,
     configure_filter/4,
-    filter/3,
+    filter/5,
     handle_db_event/3,
     handle_view_event/3,
     send_changes_doc_ids/6,
@@ -225,36 +225,60 @@ configure_filter(FilterName, Style, Req, Db) ->
             throw({bad_request, Msg})
     end.
 
-filter(Db, #full_doc_info{} = FDI, Filter) ->
-    filter(Db, couch_doc:to_doc_info(FDI), Filter);
-filter(_Db, DocInfo, {default, Style}) ->
-    apply_style(DocInfo, Style);
-filter(_Db, DocInfo, {doc_ids, Style, DocIds}) ->
-    case lists:member(DocInfo#doc_info.id, DocIds) of
-        true ->
-            apply_style(DocInfo, Style);
-        false ->
-            []
-    end;
-filter(Db, DocInfo, {selector, Style, {Selector, _Fields}}) ->
+filter(Db, #full_doc_info{} = FDI, Filter, IncludeDocs, Conflicts) ->
+    filter(Db, couch_doc:to_doc_info(FDI), Filter, IncludeDocs, Conflicts);
+filter(_Db, DocInfo, {default, Style}, _IncludeDocs, _Conflicts) ->
+    {[], apply_style(DocInfo, Style)};
+filter(_Db, DocInfo, {doc_ids, Style, DocIds}, _IncludeDocs, _Conflicts) ->
+    Revs =
+        case lists:member(DocInfo#doc_info.id, DocIds) of
+            true ->
+                apply_style(DocInfo, Style);
+            false ->
+                []
+        end,
+    {[], Revs};
+filter(_Db, DocInfo, {design_docs, Style}, _IncludeDocs, _Conflicts) ->
+    Revs =
+        case DocInfo#doc_info.id of
+            <<"_design", _/binary>> ->
+                apply_style(DocInfo, Style);
+            _ ->
+                []
+        end,
+    {[], Revs};
+filter(Db, DocInfo, {selector, all_docs, {Selector, _Fields}}, true, true) ->
+    {DocWin, Docs} = open_all_revs_include_doc(Db, DocInfo),
+    Passes = [mango_selector:match(Selector, couch_doc:to_json_obj(Doc, [])) 
|| Doc <- Docs],
+    {DocWin, filter_revs(Passes, Docs)};
+filter(Db, DocInfo, {selector, Style, {Selector, _Fields}}, IncludeDocs, 
_Conflicts) ->
     Docs = open_revs(Db, DocInfo, Style),
-    Passes = [
-        mango_selector:match(Selector, couch_doc:to_json_obj(Doc, []))
-     || Doc <- Docs
-    ],
-    filter_revs(Passes, Docs);
-filter(_Db, DocInfo, {design_docs, Style}) ->
-    case DocInfo#doc_info.id of
-        <<"_design", _/binary>> ->
-            apply_style(DocInfo, Style);
-        _ ->
-            []
+    Passes = [mango_selector:match(Selector, couch_doc:to_json_obj(Doc, [])) 
|| Doc <- Docs],
+    case IncludeDocs of
+        true -> {Docs, filter_revs(Passes, Docs)};
+        false -> {[], filter_revs(Passes, Docs)}
     end;
-filter(Db, DocInfo, {view, Style, DDoc, VName}) ->
+filter(Db, DocInfo, {view, all_docs, DDoc, VName}, true, true) ->
+    {DocWin, Docs} = open_all_revs_include_doc(Db, DocInfo),
+    {ok, Passes} = couch_query_servers:filter_view(Db, DDoc, VName, Docs),
+    {DocWin, filter_revs(Passes, Docs)};
+filter(Db, DocInfo, {view, Style, DDoc, VName}, IncludeDocs, _Conflicts) ->
     Docs = open_revs(Db, DocInfo, Style),
     {ok, Passes} = couch_query_servers:filter_view(Db, DDoc, VName, Docs),
-    filter_revs(Passes, Docs);
-filter(Db, DocInfo, {custom, Style, Req0, DDoc, FName}) ->
+    case IncludeDocs of
+        true -> {Docs, filter_revs(Passes, Docs)};
+        false -> {[], filter_revs(Passes, Docs)}
+    end;
+filter(Db, DocInfo, {custom, all_docs, Req0, DDoc, FName}, true, true) ->
+    Req =
+        case Req0 of
+            {json_req, _} -> Req0;
+            #httpd{} -> {json_req, chttpd_external:json_req_obj(Req0, Db)}
+        end,
+    {DocWin, Docs} = open_all_revs_include_doc(Db, DocInfo),
+    {ok, Passes} = couch_query_servers:filter_docs(Req, Db, DDoc, FName, Docs),
+    {DocWin, filter_revs(Passes, Docs)};
+filter(Db, DocInfo, {custom, Style, Req0, DDoc, FName}, IncludeDocs, 
_Conflicts) ->
     Req =
         case Req0 of
             {json_req, _} -> Req0;
@@ -262,7 +286,10 @@ filter(Db, DocInfo, {custom, Style, Req0, DDoc, FName}) ->
         end,
     Docs = open_revs(Db, DocInfo, Style),
     {ok, Passes} = couch_query_servers:filter_docs(Req, Db, DDoc, FName, Docs),
-    filter_revs(Passes, Docs).
+    case IncludeDocs of
+        true -> {Docs, filter_revs(Passes, Docs)};
+        false -> {[], filter_revs(Passes, Docs)}
+    end.
 
 get_view_qs({json_req, {Props}}) ->
     {Query} = couch_util:get_value(<<"query">>, Props, {[]}),
@@ -368,6 +395,17 @@ open_revs(Db, DocInfo, Style) ->
     OpenResults = [couch_db:open_doc(Db, DI, OpenOpts) || DI <- DocInfos],
     [Doc || {ok, Doc} <- OpenResults].
 
+open_all_revs_include_doc(Db, DocInfo) ->
+    DocInfos = [DocInfo#doc_info{revs = [R]} || R <- DocInfo#doc_info.revs],
+    OpenOpts = [deleted, conflicts],
+    OpenResults = [couch_db:open_doc(Db, DI, OpenOpts) || DI <- DocInfos],
+    Docs = [Doc || {ok, Doc} <- OpenResults],
+    [Doc1 | RestDocs] = Docs,
+    RestRevs = [Doc#doc.revs || Doc <- RestDocs],
+    Conflicts = [{conflicts, [{Pos, RevId} || {Pos, [RevId]} <- RestRevs]}],
+    Doc2 = Doc1#doc{meta = Conflicts},
+    {[Doc2], Docs}.
+
 filter_revs(Passes, Docs) ->
     lists:flatmap(
         fun
@@ -611,12 +649,14 @@ changes_enumerator(Value, Acc) ->
         prepend = Prepend,
         user_acc = UserAcc,
         limit = Limit,
+        include_docs = IncludeDocs,
+        conflicts = Conflicts,
         resp_type = ResponseType,
         db = Db,
         timeout = Timeout,
         timeout_fun = TimeoutFun
     } = maybe_upgrade_changes_acc(Acc),
-    Results0 = filter(Db, Value, Filter),
+    {_, Results0} = filter(Db, Value, Filter, IncludeDocs, Conflicts),
     Results = [Result || Result <- Results0, Result /= null],
     Seq =
         case Value of
diff --git a/src/fabric/src/fabric_rpc.erl b/src/fabric/src/fabric_rpc.erl
index d01f1f5a7..dab69ef4d 100644
--- a/src/fabric/src/fabric_rpc.erl
+++ b/src/fabric/src/fabric_rpc.erl
@@ -546,44 +546,57 @@ changes_enumerator(DocInfo, Acc) ->
         pending = Pending,
         epochs = Epochs
     } = Acc,
-    #doc_info{id = Id, high_seq = Seq, revs = [#rev_info{deleted = Del} | _]} 
= DocInfo,
-    case [X || X <- couch_changes:filter(Db, DocInfo, Filter), X /= null] of
-        [] ->
-            ChangesRow =
+    Opts =
+        case Conflicts of
+            true -> [conflicts | DocOptions];
+            false -> DocOptions
+        end,
+    Seq = DocInfo#doc_info.high_seq,
+    {Docs0, Revs} = couch_changes:filter(Db, DocInfo, Filter, IncludeDocs, 
Conflicts),
+    Changes = [X || X <- Revs, X /= null],
+    Docs = [X || X <- Docs0, X /= null],
+    ChangesRow =
+        case {Changes, Docs, IncludeDocs} of
+            {[], _, _} ->
                 {no_pass, [
                     {pending, Pending - 1},
                     {seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}}
                 ]};
-        Results ->
-            Opts =
-                if
-                    Conflicts -> [conflicts | DocOptions];
-                    true -> DocOptions
-                end,
-            ChangesRow =
-                {change, [
-                    {pending, Pending - 1},
-                    {seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}},
-                    {id, Id},
-                    {changes, Results},
-                    {deleted, Del}
-                    | if
-                        IncludeDocs -> [doc_member(Db, DocInfo, Opts, Filter)];
-                        true -> []
-                    end
-                ]}
-    end,
+            {_, _, false} ->
+                changes_row(Changes, [], DocInfo, Acc);
+            {_, [], true} ->
+                Docs1 = [doc_member(Db, DocInfo, Opts, Filter)],
+                changes_row(Changes, Docs1, DocInfo, Acc);
+            {_, [Doc | _], true} ->
+                Docs1 = [get_json_doc(Doc, Opts, Filter)],
+                changes_row(Changes, Docs1, DocInfo, Acc)
+        end,
     ok = rexi:stream2(ChangesRow),
     {ok, Acc#fabric_changes_acc{seq = Seq, pending = Pending - 1}}.
 
+changes_row(Changes, Docs, DocInfo, Acc) ->
+    #fabric_changes_acc{db = Db, pending = Pending, epochs = Epochs} = Acc,
+    #doc_info{id = Id, high_seq = Seq, revs = [#rev_info{deleted = Del} | _]} 
= DocInfo,
+    {change, [
+        {pending, Pending - 1},
+        {seq, {Seq, uuid(Db), couch_db:owner_of(Epochs, Seq)}},
+        {id, Id},
+        {changes, Changes},
+        {deleted, Del}
+        | Docs
+    ]}.
+
 doc_member(Shard, DocInfo, Opts, Filter) ->
     case couch_db:open_doc(Shard, DocInfo, [deleted | Opts]) of
         {ok, Doc} ->
-            {doc, maybe_filtered_json_doc(Doc, Opts, Filter)};
+            get_json_doc(Doc, Opts, Filter);
         Error ->
             Error
     end.
 
+get_json_doc(Doc, Opts, Filter) ->
+    {doc, maybe_filtered_json_doc(Doc, Opts, Filter)}.
+
 maybe_filtered_json_doc(Doc, Opts, {selector, _Style, {_Selector, Fields}}) 
when
     Fields =/= nil
 ->

Reply via email to