You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2025/07/21 21:24:22 UTC
(couchdb) branch jenkins-a-few-days-ago updated (1d05866d6 -> 4e22b4e91)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch jenkins-a-few-days-ago
in repository https://gitbox.apache.org/repos/asf/couchdb.git
discard 1d05866d6 Fix query args parsing during cluster upgrades
new 4e22b4e91 Fix query args parsing during cluster upgrades
This update added new revisions after undoing existing revisions.
That is to say, some revisions that were in the old version of the
branch are not in the new version. This situation occurs
when a user --force pushes a change and generates a repository
containing something like this:
* -- * -- B -- O -- O -- O (1d05866d6)
\
N -- N -- N refs/heads/jenkins-a-few-days-ago (4e22b4e91)
You should already have received notification emails for all of the O
revisions, and so the following emails describe only the N revisions
from the common base, B.
Any revisions marked "omit" are not gone; other references still
refer to them. Any revisions marked "discard" are gone forever.
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
Summary of changes:
(couchdb) 01/01: Fix query args parsing during cluster upgrades
Posted by va...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a commit to branch jenkins-a-few-days-ago
in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 4e22b4e9136a0d52a06371345bfa3c70cce4076e
Author: Nick Vatamaniuc <va...@gmail.com>
AuthorDate: Wed Jul 9 01:17:57 2025 -0400
Fix query args parsing during cluster upgrades
In commit caa4f9b3557e0bc6253b3099fef51903c0b5707c we optimized query args
validation so it happens on the coordinator side only. However, during online
cluster upgrades some requests like _all_docs from old nodes executed on newly
upgrades nodes will fail as the args won't be validated.
To bridge the online upgrade gap add a `validated` optional flag to args. If
the flag is not set, the workers will validate the args; if the flag is already
set by the coordinator, they won't be revalidated, so we don't lose the minor
performance gain from having to validate them twice.
---
src/couch_mrview/src/couch_mrview.erl | 14 +++++++++--
src/couch_mrview/src/couch_mrview_util.erl | 21 ++++++++++++----
.../test/eunit/couch_mrview_util_tests.erl | 28 ++++++++++++++++++++++
src/fabric/test/eunit/fabric_tests.erl | 2 +-
4 files changed, 58 insertions(+), 7 deletions(-)
diff --git a/src/couch_mrview/src/couch_mrview.erl b/src/couch_mrview/src/couch_mrview.erl
index 037c35cdd..0ae313e7c 100644
--- a/src/couch_mrview/src/couch_mrview.erl
+++ b/src/couch_mrview/src/couch_mrview.erl
@@ -287,12 +287,22 @@ query_all_docs(Db, Args0, Callback, Acc) ->
couch_index_util:hexsig(couch_hash:md5_hash(?term_to_bin(Info)))
end),
Args1 = Args0#mrargs{view_type = map},
+
+ % TODO: Compatibility clause. Remove after upgrading to next minor release
+ % after 3.5.0.
+ %
+ % As of commit 7aa8a4, args are validated in fabric. However, during online
+ % cluster upgrades, old nodes will still expect args to be validated on
+ % workers, so keep the clause around until the next minor version then
+ % remove.
+ %
+ Args2 = couch_mrview_util:validate_all_docs_args(Db, Args1),
{ok, Acc1} =
- case Args1#mrargs.preflight_fun of
+ case Args2#mrargs.preflight_fun of
PFFun when is_function(PFFun, 2) -> PFFun(Sig, Acc);
_ -> {ok, Acc}
end,
- all_docs_fold(Db, Args1, Callback, Acc1).
+ all_docs_fold(Db, Args2, Callback, Acc1).
query_view(Db, DDoc, VName) ->
Args = #mrargs{extra = [{view_row_map, true}]},
diff --git a/src/couch_mrview/src/couch_mrview_util.erl b/src/couch_mrview/src/couch_mrview_util.erl
index a5c81a073..03e35687e 100644
--- a/src/couch_mrview/src/couch_mrview_util.erl
+++ b/src/couch_mrview/src/couch_mrview_util.erl
@@ -544,7 +544,13 @@ apply_limit(ViewPartitioned, Args) ->
mrverror(io_lib:format(Fmt, [MaxLimit]))
end.
-validate_all_docs_args(Db, Args0) ->
+validate_all_docs_args(Db, #mrargs{} = Args) ->
+ case get_extra(Args, validated, false) of
+ true -> Args;
+ false -> validate_all_docs_args_int(Db, Args)
+ end.
+
+validate_all_docs_args_int(Db, Args0) ->
Args = validate_args(Args0#mrargs{view_type = map}),
DbPartitioned = couch_db:is_partitioned(Db),
@@ -560,7 +566,13 @@ validate_all_docs_args(Db, Args0) ->
apply_limit(false, Args)
end.
-validate_args(Args) ->
+validate_args(#mrargs{} = Args) ->
+ case get_extra(Args, validated, false) of
+ true -> Args;
+ false -> validate_args_int(Args)
+ end.
+
+validate_args_int(#mrargs{} = Args) ->
GroupLevel = determine_group_level(Args),
Reduce = Args#mrargs.reduce,
case Reduce == undefined orelse is_boolean(Reduce) of
@@ -696,11 +708,12 @@ validate_args(Args) ->
_ -> mrverror(<<"Invalid value for `partition`.">>)
end,
- Args#mrargs{
+ Args1 = Args#mrargs{
start_key_docid = SKDocId,
end_key_docid = EKDocId,
group_level = GroupLevel
- }.
+ },
+ set_extra(Args1, validated, true).
determine_group_level(#mrargs{group = undefined, group_level = undefined}) ->
0;
diff --git a/src/couch_mrview/test/eunit/couch_mrview_util_tests.erl b/src/couch_mrview/test/eunit/couch_mrview_util_tests.erl
index 2562bb511..c304dcdad 100644
--- a/src/couch_mrview/test/eunit/couch_mrview_util_tests.erl
+++ b/src/couch_mrview/test/eunit/couch_mrview_util_tests.erl
@@ -174,3 +174,31 @@ t_get_index_files_clustered({DbName, _Db}) ->
?assertMatch({ok, _}, file:read_file_info(File)),
{ok, Info} = couch_mrview:get_info(ShardName1, ?DDOC_ID),
?assertEqual(proplists:get_value(signature, Info), Sig).
+
+do_not_validate_args_if_already_validated_test() ->
+ Args = #mrargs{
+ view_type = red,
+ group = true,
+ group_level = undefined,
+ extra = [{foo, bar}]
+ },
+
+ % Initially if we haven't validated, it's not flagged as such
+ ?assertNot(couch_mrview_util:get_extra(Args, validated, false)),
+
+ % Do the validation
+ Args1 = couch_mrview_util:validate_args(Args),
+
+ % Validation worked
+ ?assertEqual(exact, Args1#mrargs.group_level),
+
+ % Validation flag is set to true
+ ?assert(couch_mrview_util:get_extra(Args1, validated, false)),
+
+ Args2 = couch_mrview_util:validate_args(Args1),
+ % No change, as already validated
+ ?assertEqual(Args1, Args2),
+
+ Args3 = couch_mrview_util:validate_all_docs_args(some_db, Args2),
+ % No change for all docs validation as already validated
+ ?assertEqual(Args1, Args3).
diff --git a/src/fabric/test/eunit/fabric_tests.erl b/src/fabric/test/eunit/fabric_tests.erl
index 1ba5d1bc6..2788af19e 100644
--- a/src/fabric/test/eunit/fabric_tests.erl
+++ b/src/fabric/test/eunit/fabric_tests.erl
@@ -298,7 +298,7 @@ t_query_view_configuration({_Ctx, DbName}) ->
view_type = map,
start_key_docid = <<>>,
end_key_docid = <<255>>,
- extra = [{view_row_map, true}]
+ extra = [{validated, true}, {view_row_map, true}]
},
Options = [],
Accumulator = [],