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 2023/01/19 14:44:14 UTC
[GitHub] [couchdb] mikerhodes opened a new pull request, #4394: Mango fields pushdown
mikerhodes opened a new pull request, #4394:
URL: https://github.com/apache/couchdb/pull/4394
## Overview
This PR aims to improve Mango by reducing the data transferred to
the coordinator during query execution. It may reduce memory or CPU use
at the coordinator but that isn't the primary goal.
Currently, when documents are read at the shard level, they are compared
locally at the shard with the selector to ensure they match before they
are sent to the coordinator. This ensures we're not sending documents
across the network that the coordinator immediately discards, saving
bandwidth and coordinator processing. This PR further executes field
projection (`fields` in the query) at the shard level. This should
further save bandwidth, particularly for queries that project few fields
from large documents.
One item of complexity is that a query may request a quorum read of
documents, meaning that we need to do the document read at the
coordinator and not the shard, then perform the `selector` and `fields`
processing there rather than at the shard. To ensure that documents are
processed consistently whether at the shard or coordinator,
match_and_extract_doc/3 is added. There is still one orphan call outside
match_and_extract_doc/2 to extract/2 which supports cluster upgrade and
should later be removed.
Shard level processing is already performed in a callback, view_cb/2,
that's passed to fabric's view processing to run for each row in the
view result set. It's used for the shard local selector and fields
processing. To make it clear what arguments are destined for this
callback, the PR encapsulates the arguments, using viewcbargs_new/2
and viewcbargs_get/2.
As we push down more functionality to the shard, the context this
function needs to carry with it will increase, so having a record for it
will be valuable.
Supporting cluster upgrades:
The PR supports shard pushdown for Mango `fields` processing for
situations during rolling cluster upgrades. (Cloudant require this
as they use rolling upgrades).
In the state where the coordinator is speaking to an upgraded node, the
view_cb/2 needs to support being passed just the `selector` outside of
the new viewcbargs record. In this case, the shard will not process
fields, but the coordinator will.
In the situation where the coordinator is upgraded but the shard is not,
we need to send the selector to the shard via `selector` and also
execute the fields projection at the coordinator. Therefore we pass
arguments to view_cb/2 via both `selector` and `callback_args` and have
an apparently spurious field projection (mango_fields:extract/2) in the
code that receives back values from the shard ( factored out into
doc_member_and_extract).
Both of these affordances should only need to exist through one minor
version change and be removed thereafter -- if people are jumping
several minor versions of CouchDB in one go, hopefully they are prepared
for a bit of trouble.
Testing upgrade states:
As view_cb is completely separate from the rest of the cursor code,
we can first try out the branch's code using view_cb from `main`, and
then the other way -- the branch's view_cb with the rest of the file
from main. I did both of these tests successfully.
## Testing recommendations
This PR should not change anything from an end user perspective. Mango responses should remain the same as they currently are.
I have run some basic performance locally tests using k6.io, which showed no meaningful change in the latency of requests.
## Related Issues or Pull Requests
none.
## Checklist
- [x] Code is written and works correctly
- [x] Changes are covered by tests
- [ ] Any new configurable parameters are documented in `rel/overlay/etc/default.ini`
- [ ] Documentation changes were made in the `src/docs` folder
- [ ] Documentation changes were backported (separated PR) to affected branches
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081599218
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -35,6 +35,19 @@
-define(HEARTBEAT_INTERVAL_IN_USEC, 4000000).
+% viewcbargs wraps up the arguments that view_cb uses into a single
+% entry in the mrargs.extra list. We use a Map to allow us to later
+% add fields without having old messages causing errors/crashes.
+viewcbargs_new(Selector, Fields) ->
+ #{
+ selector => Selector,
+ fields => Fields
+ }.
+viewcbargs_get(selector, Args) when is_map(Args) ->
Review Comment:
I'm going to leave this one as is for now. Let me know if you'd rather I change it.
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082067804
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -422,16 +472,21 @@ apply_opts([{_, _} | Rest], Args) ->
% Ignore unknown options
apply_opts(Rest, Args).
-doc_member(Cursor, RowProps) ->
+doc_member_and_extract(Cursor, RowProps) ->
Db = Cursor#cursor.db,
Opts = Cursor#cursor.opts,
ExecutionStats = Cursor#cursor.execution_stats,
Selector = Cursor#cursor.selector,
case couch_util:get_value(doc, RowProps) of
{DocProps} ->
- % only matching documents are returned; the selector
- % is evaluated at the shard level in view_cb({row, Row},
- {ok, {DocProps}, {execution_stats, ExecutionStats}};
+ % If the query doesn't request quorum doc read via r>1,
+ % match_and_extract_doc/3 is executed in in view_cb, ie, locally
Review Comment:
two `in`s in `is executed in in view_cb`
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082305003
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -441,7 +496,12 @@ doc_member(Cursor, RowProps) ->
case mango_util:defer(fabric, open_doc, [Db, Id, Opts]) of
{ok, #doc{} = DocProps} ->
Doc = couch_doc:to_json_obj(DocProps, []),
- match_doc(Selector, Doc, ExecutionStats1);
+ case match_and_extract_doc(Doc, Selector, Cursor#cursor.fields) of
+ {match, FinalDoc} ->
+ {ok, FinalDoc, {execution_stats, ExecutionStats1}};
+ {no_match, _} ->
Review Comment:
There's no reason to leave it unspecified. I wasn't sure whether to go for more "fixed" pattern matching or be freer, so I'm more than happy to change. I'll update and squash in the change to the existing commit.
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081539012
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
+ {Selector, Fields} =
+ case couch_util:get_value(callback_args, Options) of
+ % old style
+ undefined ->
+ {couch_util:get_value(selector, Options), undefined};
+ % new style - assume a viewcbargs
+ Args ->
Review Comment:
I wanted to avoid exposing the "internals" of the viewcbargs here. I found you couldn't use your own function in a guard only a BIF, ie, I tried `when is_viewcbargs(Args)`. In the end, perhaps you're right that it's better to leak the Map implementation such that there is a decent guard on the `case` statement -- what do you think, I'm not sure what works well in Erlang here?
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082853325
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
We've been informally tagging those with "backwards compatibility" or "upgrade clause" notes in the comments. Searching for "TODO" would work as well. Some official marker would be better, of course.
Another way is to add a config parameter like we had for rexi use_kill_all but there is a balance there.
But I am inclined to merge it as is and do the extra formalizing as another task
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082069624
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -441,7 +496,12 @@ doc_member(Cursor, RowProps) ->
case mango_util:defer(fabric, open_doc, [Db, Id, Opts]) of
{ok, #doc{} = DocProps} ->
Doc = couch_doc:to_json_obj(DocProps, []),
- match_doc(Selector, Doc, ExecutionStats1);
+ case match_and_extract_doc(Doc, Selector, Cursor#cursor.fields) of
+ {match, FinalDoc} ->
+ {ok, FinalDoc, {execution_stats, ExecutionStats1}};
+ {no_match, _} ->
Review Comment:
Should we assert `{no_match, undefined}` since we know it should be only that as the `no_match` choice? Not a big deal either way but it's nicer to have stronger asserts when possible, unless there is a reason leave it `undefined` (future compatibility?)
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082853325
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
We've been informally tagging those with "backwards compatibility" or "upgrade clause" notes in the comments. Searching for "TODO" would work as well. Some official marker would be better, of course.
Another way is to add a config parameter like we had for rexi use_kill_all but there is a balance there.
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082582977
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
This seems reasonable. My question is: does this removal need to wait for 4.x, given it's not a breaking change, or does it need t wait a couple of minor - or patch - releases?
I don't know how many people do rolling upgrades outside of Cloudant, as opposed to just stopping the world for a few minutes.
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081579998
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
+ {Selector, Fields} =
+ case couch_util:get_value(callback_args, Options) of
+ % old style
+ undefined ->
+ {couch_util:get_value(selector, Options), undefined};
+ % new style - assume a viewcbargs
+ Args ->
Review Comment:
It's a balance, but I think it's slightly better to leak some of the implementation for a stronger assert in the case. It also tells the reader what the types are in that snippet right away ("we are dealing with a map or undefined only") so serves as a shorthand reminder in a way.
--
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] janl commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
janl commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082607599
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
I think enough folks do this and not necessarily within a small round of point releases that would warrant keeping it up until 4.0.0
--
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] janl commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
janl commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082660139
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
but Iām fine with making this change in a follow-up PR where we go through all our rolling update compat code and merge this as-is #ScopeCreep
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082822189
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
Okay. Sounds good.
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081514712
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -35,6 +35,19 @@
-define(HEARTBEAT_INTERVAL_IN_USEC, 4000000).
+% viewcbargs wraps up the arguments that view_cb uses into a single
+% entry in the mrargs.extra list. We use a Map to allow us to later
+% add fields without having old messages causing errors/crashes.
+viewcbargs_new(Selector, Fields) ->
+ #{
+ selector => Selector,
+ fields => Fields
+ }.
+viewcbargs_get(selector, Args) when is_map(Args) ->
Review Comment:
Another alternative we typically use is `viewcbargs_get(selector, #{} = Args)` but either way if fine.
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082305003
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -441,7 +496,12 @@ doc_member(Cursor, RowProps) ->
case mango_util:defer(fabric, open_doc, [Db, Id, Opts]) of
{ok, #doc{} = DocProps} ->
Doc = couch_doc:to_json_obj(DocProps, []),
- match_doc(Selector, Doc, ExecutionStats1);
+ case match_and_extract_doc(Doc, Selector, Cursor#cursor.fields) of
+ {match, FinalDoc} ->
+ {ok, FinalDoc, {execution_stats, ExecutionStats1}};
+ {no_match, _} ->
Review Comment:
There's no reason to leave it unspecified. I wasn't sure whether to go for more "fixed" pattern matching or be freer, so I'm more than happy to change. I'll update and squash in a new commit.
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081520232
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
+ {Selector, Fields} =
+ case couch_util:get_value(callback_args, Options) of
+ % old style
+ undefined ->
+ {couch_util:get_value(selector, Options), undefined};
+ % new style - assume a viewcbargs
+ Args ->
Review Comment:
We will explicitly match `Args` to be a map in `viewcbargs_get(selector, Args)` but it won't hurt to also match it here as `Args = #{}`. Just to make it clear in this part of the code what we're dealing with.
--
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] janl merged pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
janl merged PR #4394:
URL: https://github.com/apache/couchdb/pull/4394
--
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 #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
nickva commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082069624
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -441,7 +496,12 @@ doc_member(Cursor, RowProps) ->
case mango_util:defer(fabric, open_doc, [Db, Id, Opts]) of
{ok, #doc{} = DocProps} ->
Doc = couch_doc:to_json_obj(DocProps, []),
- match_doc(Selector, Doc, ExecutionStats1);
+ case match_and_extract_doc(Doc, Selector, Cursor#cursor.fields) of
+ {match, FinalDoc} ->
+ {ok, FinalDoc, {execution_stats, ExecutionStats1}};
+ {no_match, _} ->
Review Comment:
Should we assert `{no_match, undefined}` since we know it should be only that as the `no_match` choice?
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1081596322
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
+ {Selector, Fields} =
+ case couch_util:get_value(callback_args, Options) of
+ % old style
+ undefined ->
+ {couch_util:get_value(selector, Options), undefined};
+ % new style - assume a viewcbargs
+ Args ->
Review Comment:
Added the pattern match to the Map as suggested. Squashed into the original commit.
--
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] mikerhodes commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
mikerhodes commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082300742
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -422,16 +472,21 @@ apply_opts([{_, _} | Rest], Args) ->
% Ignore unknown options
apply_opts(Rest, Args).
-doc_member(Cursor, RowProps) ->
+doc_member_and_extract(Cursor, RowProps) ->
Db = Cursor#cursor.db,
Opts = Cursor#cursor.opts,
ExecutionStats = Cursor#cursor.execution_stats,
Selector = Cursor#cursor.selector,
case couch_util:get_value(doc, RowProps) of
{DocProps} ->
- % only matching documents are returned; the selector
- % is evaluated at the shard level in view_cb({row, Row},
- {ok, {DocProps}, {execution_stats, ExecutionStats}};
+ % If the query doesn't request quorum doc read via r>1,
+ % match_and_extract_doc/3 is executed in in view_cb, ie, locally
Review Comment:
š fixed and squashed.
--
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] janl commented on a diff in pull request #4394: Mango fields pushdown
Posted by GitBox <gi...@apache.org>.
janl commented on code in PR #4394:
URL: https://github.com/apache/couchdb/pull/4394#discussion_r1082422633
##########
src/mango/src/mango_cursor_view.erl:
##########
@@ -248,6 +266,19 @@ view_cb({row, Row}, #mrargs{extra = Options} = Acc) ->
key = couch_util:get_value(key, Row),
doc = couch_util:get_value(doc, Row)
},
+ % This supports receiving our "arguments" either as just the `selector`
+ % or in the new record in `callback_args`. This is to support mid-upgrade
+ % clusters where the non-upgraded coordinator nodes will send the older style.
+ % TODO remove this in a couple of couchdb versions.
Review Comment:
might be worth codifying this a little. I know we have other places, but maybe e can start here:
maybe we can make a comment like `% x-couch-remove: 4.0.0` that we can then grep for prior to releasing 4.0.0.
The reasoning here that we can say: upgrade everything to the latest 3.x version and THEN go to 4.0.0 will be smooth, rather than supporting any 3.x-> 4.0.0
--
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