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/09/12 16:56:38 UTC
(couchdb) branch unify-metadata-setters created (now 5d7540eb5)
This is an automated email from the ASF dual-hosted git repository.
vatamane pushed a change to branch unify-metadata-setters
in repository https://gitbox.apache.org/repos/asf/couchdb.git
at 5d7540eb5 Unify and fix fabric db meta setters
This branch includes the following new commits:
new 5d7540eb5 Unify and fix fabric db meta setters
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.
(couchdb) 01/01: Unify and fix fabric db meta setters
Posted by va...@apache.org.
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 <va...@gmail.com>
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).