This is an automated email from the ASF dual-hosted git repository. vatamane pushed a commit to branch unify-metadata-setters in repository https://gitbox.apache.org/repos/asf/couchdb.git
commit 5d7540eb5cd129b00c03505434ceb67e485c37d0 Author: Nick Vatamaniuc <[email protected]> AuthorDate: Fri Sep 12 12:43:10 2025 -0400 Unify and fix fabric db meta setters Previously, `set_revs_limit`, `set_purge_infos_limit` and `update_props` did essentially the same steps and just differently named handlers. So clean up and make the mall use same underlying function. To simplfy things even further, use the `fabric:ring_handle*`. There is already an `all` ring option to handle sending the message to all the shards and handling rexi downs and exits. Add tests for all the functions in fabric_db_meta module. Thanks to Jessica (@jiahuili430) for pointing out the lack of rexi down and exit error handling in fabric meta setters. --- src/fabric/src/fabric_db_meta.erl | 90 ++++++++----------- src/fabric/test/eunit/fabric_db_info_tests.erl | 22 +---- src/fabric/test/eunit/fabric_meta_tests.erl | 117 +++++++++++++++++++++++++ 3 files changed, 155 insertions(+), 74 deletions(-) diff --git a/src/fabric/src/fabric_db_meta.erl b/src/fabric/src/fabric_db_meta.erl index af4a069d4..b87820fef 100644 --- a/src/fabric/src/fabric_db_meta.erl +++ b/src/fabric/src/fabric_db_meta.erl @@ -20,7 +20,6 @@ update_props/4 ]). --include_lib("fabric/include/fabric.hrl"). -include_lib("mem3/include/mem3.hrl"). -include_lib("couch/include/couch_db.hrl"). @@ -31,48 +30,13 @@ }). set_revs_limit(DbName, Limit, Options) -> - Shards = mem3:shards(DbName), - Workers = fabric_util:submit_jobs(Shards, set_revs_limit, [Limit, Options]), - Handler = fun handle_revs_message/3, - Acc0 = {Workers, length(Workers) - 1}, - case fabric_util:recv(Workers, #shard.ref, Handler, Acc0) of - {ok, ok} -> - ok; - {timeout, {DefunctWorkers, _}} -> - fabric_util:log_timeout(DefunctWorkers, "set_revs_limit"), - {error, timeout}; - Error -> - Error - end. - -handle_revs_message(ok, _, {_Workers, 0}) -> - {stop, ok}; -handle_revs_message(ok, Worker, {Workers, Waiting}) -> - {ok, {lists:delete(Worker, Workers), Waiting - 1}}; -handle_revs_message(Error, _, _Acc) -> - {error, Error}. + set_meta(DbName, set_revs_limit, [Limit, Options]). set_purge_infos_limit(DbName, Limit, Options) -> - Shards = mem3:shards(DbName), - Workers = fabric_util:submit_jobs(Shards, set_purge_infos_limit, [Limit, Options]), - Handler = fun handle_purge_message/3, - Acc0 = {Workers, length(Workers) - 1}, - case fabric_util:recv(Workers, #shard.ref, Handler, Acc0) of - {ok, ok} -> - ok; - {timeout, {DefunctWorkers, _}} -> - fabric_util:log_timeout(DefunctWorkers, "set_purged_docs_limit"), - {error, timeout}; - Error -> - Error - end. + set_meta(DbName, set_purge_infos_limit, [Limit, Options]). -handle_purge_message(ok, _, {_Workers, 0}) -> - {stop, ok}; -handle_purge_message(ok, Worker, {Workers, Waiting}) -> - {ok, {lists:delete(Worker, Workers), Waiting - 1}}; -handle_purge_message(Error, _, _Acc) -> - {error, Error}. +update_props(DbName, K, V, Options) -> + set_meta(DbName, update_props, [K, V, Options]). set_security(DbName, SecObj, Options) -> Shards = mem3:shards(DbName), @@ -200,24 +164,44 @@ maybe_finish_get(#acc{workers = []} = Acc) -> maybe_finish_get(Acc) -> {ok, Acc}. -update_props(DbName, K, V, Options) -> +set_meta(DbName, Fun, Args) when is_atom(Fun), is_list(Args) -> Shards = mem3:shards(DbName), - Workers = fabric_util:submit_jobs(Shards, update_props, [K, V, Options]), - Handler = fun handle_update_props_message/3, - Acc0 = {Workers, length(Workers) - 1}, - case fabric_util:recv(Workers, #shard.ref, Handler, Acc0) of + Workers = fabric_util:submit_jobs(Shards, Fun, Args), + RexiMon = fabric_util:create_monitors(Shards), + Acc0 = {fabric_dict:init(Workers, nil), []}, + try fabric_util:recv(Workers, #shard.ref, fun handle_set_meta_message/3, Acc0) of {ok, ok} -> ok; - {timeout, {DefunctWorkers, _}} -> - fabric_util:log_timeout(DefunctWorkers, "update_props"), + {timeout, {WorkersDict, _}} -> + DefunctWorkers = fabric_util:remove_done_workers(WorkersDict, nil), + fabric_util:log_timeout(DefunctWorkers, atom_to_list(Fun)), {error, timeout}; Error -> Error + after + rexi_monitor:stop(RexiMon) end. -handle_update_props_message(ok, _, {_Workers, 0}) -> - {stop, ok}; -handle_update_props_message(ok, Worker, {Workers, Waiting}) -> - {ok, {lists:delete(Worker, Workers), Waiting - 1}}; -handle_update_props_message(Error, _, _Acc) -> - {error, Error}. +handle_set_meta_message({rexi_DOWN, _, {_, NodeRef}, _}, _Shard, {Cntrs, Res}) -> + case fabric_ring:node_down(NodeRef, Cntrs, Res, [all]) of + {ok, Cntrs1} -> {ok, {Cntrs1, Res}}; + error -> {error, {nodedown, <<"progress not possible">>}} + end; +handle_set_meta_message({rexi_EXIT, Reason}, Shard, {Cntrs, Res}) -> + case fabric_ring:handle_error(Shard, Cntrs, Res, [all]) of + {ok, Cntrs1} -> {ok, {Cntrs1, Res}}; + error -> {error, Reason} + end; +handle_set_meta_message(ok, Worker, {Cntrs, Res}) -> + case fabric_ring:handle_response(Worker, ok, Cntrs, Res, [all]) of + {ok, {Cntrs1, Res1}} -> + {ok, {Cntrs1, Res1}}; + {stop, _Res1} -> + % We only stored ok results so we just return ok + {stop, ok} + end; +handle_set_meta_message(Reason, Shard, {Cntrs, Res}) -> + case fabric_ring:handle_error(Shard, Cntrs, Res, [all]) of + {ok, Cntrs1} -> {ok, {Cntrs1, Res}}; + error -> {error, Reason} + end. diff --git a/src/fabric/test/eunit/fabric_db_info_tests.erl b/src/fabric/test/eunit/fabric_db_info_tests.erl index 9a133ace5..d662e390e 100644 --- a/src/fabric/test/eunit/fabric_db_info_tests.erl +++ b/src/fabric/test/eunit/fabric_db_info_tests.erl @@ -69,25 +69,5 @@ t_update_and_get_props(_) -> {ok, Info1} = fabric:get_db_info(DbName), Props1 = couch_util:get_value(props, Info1), ?assertEqual({[{<<"foo">>, 100}]}, Props1), - - ?assertEqual(ok, fabric:update_props(DbName, bar, 101)), - {ok, Info2} = fabric:get_db_info(DbName), - Props2 = couch_util:get_value(props, Info2), - ?assertEqual( - {[ - {<<"foo">>, 100}, - {bar, 101} - ]}, - Props2 - ), - - ?assertEqual(ok, fabric:update_props(DbName, <<"foo">>, undefined)), - {ok, Info3} = fabric:get_db_info(DbName), - ?assertEqual({[{bar, 101}]}, couch_util:get_value(props, Info3)), - - Res = fabric:update_props(DbName, partitioned, true), - ?assertMatch({error, {bad_request, _}}, Res), - {ok, Info4} = fabric:get_db_info(DbName), - ?assertEqual({[{bar, 101}]}, couch_util:get_value(props, Info4)), - + % There are more update_props tests in fabric_meta_tests module ok = fabric:delete_db(DbName, []). diff --git a/src/fabric/test/eunit/fabric_meta_tests.erl b/src/fabric/test/eunit/fabric_meta_tests.erl new file mode 100644 index 000000000..cb568fa66 --- /dev/null +++ b/src/fabric/test/eunit/fabric_meta_tests.erl @@ -0,0 +1,117 @@ +% Licensed under the Apache License, Version 2.0 (the "License"); you may not +% use this file except in compliance with the License. You may obtain a copy of +% the License at +% +% http://www.apache.org/licenses/LICENSE-2.0 +% +% Unless required by applicable law or agreed to in writing, software +% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT +% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the +% License for the specific language governing permissions and limitations under +% the License. + +-module(fabric_meta_tests). + +-include_lib("couch/include/couch_eunit.hrl"). +-include_lib("couch/include/couch_db.hrl"). + +fabric_meta_test_() -> + { + setup, + fun setup/0, + fun teardown/1, + with([ + ?TDEF(t_set_revs_limit), + ?TDEF(t_set_purge_revs_limit), + ?TDEF(t_update_props), + ?TDEF(t_security) + ]) + }. + +setup() -> + test_util:start_couch([fabric]). + +teardown(Ctx) -> + test_util:stop_couch(Ctx). + +t_update_props(_) -> + DbName = ?tempdb(), + ok = fabric:create_db(DbName, [{q, 2}, {n, 1}]), + + {ok, Info} = fabric:get_db_info(DbName), + Props = couch_util:get_value(props, Info), + ?assertEqual({[]}, Props), + + ?assertEqual(ok, fabric:update_props(DbName, <<"foo">>, 100)), + {ok, Info1} = fabric:get_db_info(DbName), + Props1 = couch_util:get_value(props, Info1), + % 200 because q=2 and we're using get_db_info which sums + % the info object integers when merging them + ?assertEqual({[{<<"foo">>, 200}]}, Props1), + + ?assertEqual(ok, fabric:update_props(DbName, bar, 101)), + {ok, Info2} = fabric:get_db_info(DbName), + Props2 = couch_util:get_value(props, Info2), + ?assertEqual( + {[ + {<<"foo">>, 200}, + {bar, 202} + ]}, + Props2 + ), + + ?assertEqual(ok, fabric:update_props(DbName, <<"foo">>, undefined)), + {ok, Info3} = fabric:get_db_info(DbName), + ?assertEqual({[{bar, 202}]}, couch_util:get_value(props, Info3)), + + Res = fabric:update_props(DbName, partitioned, true), + ?assertMatch({error, {bad_request, _}}, Res), + {ok, Info4} = fabric:get_db_info(DbName), + ?assertEqual({[{bar, 202}]}, couch_util:get_value(props, Info4)), + + ok = fabric:delete_db(DbName, []). + +t_set_revs_limit(_) -> + DbName = ?tempdb(), + ok = fabric:create_db(DbName, [{q, 2}, {n, 1}]), + ?assertEqual(ok, fabric:set_revs_limit(DbName, 3, [?ADMIN_CTX])), + Check = fun(Db) -> + ?assertEqual(3, couch_db:get_revs_limit(Db)) + end, + [check_shard(S, Check) || S <- mem3:shards(DbName)]. + +t_set_purge_revs_limit(_) -> + DbName = ?tempdb(), + ok = fabric:create_db(DbName, [{q, 2}, {n, 1}]), + ?assertEqual(ok, fabric:set_purge_infos_limit(DbName, 3, [?ADMIN_CTX])), + Check = fun(Db) -> + ?assertEqual(3, couch_db:get_purge_infos_limit(Db)) + end, + [check_shard(S, Check) || S <- mem3:shards(DbName)]. + +t_security(_) -> + DbName = ?tempdb(), + ok = fabric:create_db(DbName, [{q, 2}, {n, 1}]), + SecObj = #{ + <<"admins">> => #{<<"names">> => [<<"n1">>], <<"roles">> => [<<"r1">>]}, + <<"members">> => #{<<"names">> => [<<"n2">>], <<"roles">> => [<<"r2">>]} + }, + Ejson = ?JSON_DECODE(?JSON_ENCODE(SecObj)), + ?assertEqual(ok, fabric:set_security(DbName, Ejson)), + Check = fun(Db) -> + ?assertEqual(SecObj, couch_util:ejson_to_map(couch_db:get_security(Db))) + end, + [check_shard(S, Check) || S <- mem3:shards(DbName)], + + AllSec = fabric:get_all_security(DbName), + ?assertMatch({ok, _}, AllSec), + {ok, ShardSec} = AllSec, + ?assert(is_list(ShardSec)), + ?assertEqual(2, length(ShardSec)), + % ShardSec result is [{#shard{}, SecObj}, ...] + {_, SecObjs} = lists:unzip(ShardSec), + [?assertEqual(SecObj, couch_util:ejson_to_map(O)) || O <- SecObjs]. + +check_shard(Shard, Check) -> + Name = mem3:name(Shard), + couch_util:with_db(Name, Check).
