This is an automated email from the ASF dual-hosted git repository. vatamane pushed a commit to branch online-upgrade-query-param-fix in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit c800c325d6d8411c341d09809960f6f43910ce27 Author: Nick Vatamaniuc <[email protected]> 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 = [],
