diff options
author | Paul J. Davis <paul.joseph.davis@gmail.com> | 2018-06-15 11:44:20 -0500 |
---|---|---|
committer | Paul J. Davis <paul.joseph.davis@gmail.com> | 2018-06-15 13:39:18 -0500 |
commit | 000766c4f8b51562757818dfa36b3e6579d16309 (patch) | |
tree | e9ae5d29069fbd958c0a694b9290e502db410733 | |
parent | c7d35c3133499fcb2f7989aea0c4c0e9277351f9 (diff) | |
download | couchdb-000766c4f8b51562757818dfa36b3e6579d16309.tar.gz |
Fix active size calculations for views
It was brought to my attention that the active size looked a bit funny
occasionally as it could be larger than the file size. Given that active
size is defined to be the number of bytes used in a view file it must be
strictly greater than or equal to zero as well as less than or equal to
file size. Thus the great hunt had begun!
While I'd love more than anything to regail you, Dear Reader, with the
tales, the joys, the highs, and yes, even the lows of this search, it
turns out that I cannot. I must not. For there were none!
Turns out this was a trivial bug where we were re-using the ExternalSize
calculation instead of applying `couch_btree:size/1` to all of the
btrees in the `#mrview` records. Simple bug comes with a correspondingly
simple fix.
I also noticed that the info tests were broken and not being run so I
spent a few minutes cleaning those up to make the various assumptions.
-rw-r--r-- | src/couch_mrview/src/couch_mrview_index.erl | 3 | ||||
-rw-r--r-- | src/couch_mrview/src/couch_mrview_util.erl | 17 | ||||
-rw-r--r-- | src/couch_mrview/test/couch_mrview_index_info_tests.erl | 96 |
3 files changed, 90 insertions, 26 deletions
diff --git a/src/couch_mrview/src/couch_mrview_index.erl b/src/couch_mrview/src/couch_mrview_index.erl index aa1ee2741..5d285d639 100644 --- a/src/couch_mrview/src/couch_mrview_index.erl +++ b/src/couch_mrview/src/couch_mrview_index.erl @@ -61,13 +61,14 @@ get(info, State) -> } = State, {ok, FileSize} = couch_file:bytes(Fd), {ok, ExternalSize} = couch_mrview_util:calculate_external_size(Views), + {ok, ActiveViewSize} = couch_mrview_util:calculate_active_size(Views), LogBtSize = case LogBtree of nil -> 0; _ -> couch_btree:size(LogBtree) end, - ActiveSize = couch_btree:size(IdBtree) + LogBtSize + ExternalSize, + ActiveSize = couch_btree:size(IdBtree) + LogBtSize + ActiveViewSize, UpdateOptions0 = get(update_options, State), UpdateOptions = [atom_to_binary(O, latin1) || O <- UpdateOptions0], diff --git a/src/couch_mrview/src/couch_mrview_util.erl b/src/couch_mrview/src/couch_mrview_util.erl index 0c6e5fc88..eb461d017 100644 --- a/src/couch_mrview/src/couch_mrview_util.erl +++ b/src/couch_mrview/src/couch_mrview_util.erl @@ -23,6 +23,7 @@ -export([fold/4, fold_reduce/4]). -export([temp_view_to_ddoc/1]). -export([calculate_external_size/1]). +-export([calculate_active_size/1]). -export([validate_args/1]). -export([maybe_load_doc/3, maybe_load_doc/4]). -export([maybe_update_index_file/1]). @@ -830,6 +831,22 @@ calculate_external_size(Views) -> {ok, lists:foldl(SumFun, 0, Views)}. +calculate_active_size(Views) -> + BtSize = fun + (nil) -> 0; + (Bt) -> couch_btree:size(Bt) + end, + FoldFun = fun(View, Acc) -> + Sizes = [ + BtSize(View#mrview.btree), + BtSize(View#mrview.seq_btree), + BtSize(View#mrview.key_byseq_btree) + ], + Acc + lists:sum([S || S <- Sizes, is_integer(S)]) + end, + {ok, lists:foldl(FoldFun, 0, Views)}. + + sum_btree_sizes(nil, _) -> 0; sum_btree_sizes(_, nil) -> diff --git a/src/couch_mrview/test/couch_mrview_index_info_tests.erl b/src/couch_mrview/test/couch_mrview_index_info_tests.erl index c994df9d3..efa03e7c0 100644 --- a/src/couch_mrview/test/couch_mrview_index_info_tests.erl +++ b/src/couch_mrview/test/couch_mrview_index_info_tests.erl @@ -18,14 +18,13 @@ -define(TIMEOUT, 1000). --ifdef(run_broken_tests). - setup() -> {ok, Db} = couch_mrview_test_util:init_db(?tempdb(), map), couch_mrview:query_view(Db, <<"_design/bar">>, <<"baz">>), {ok, Info} = couch_mrview:get_info(Db, <<"_design/bar">>), {Db, Info}. + teardown({Db, _}) -> couch_db:close(Db), couch_server:delete(couch_db:name(Db), [?ADMIN_CTX]), @@ -37,39 +36,86 @@ view_info_test_() -> "Views index tests", { setup, - fun test_util:start_couch/0, fun test_util:stop_couch/1, + fun test_util:start_couch/0, + fun test_util:stop_couch/1, { foreach, - fun setup/0, fun teardown/1, + fun setup/0, + fun teardown/1, [ - fun should_get_property/1 + fun sig_is_binary/1, + fun language_is_js/1, + fun file_size_is_non_neg_int/1, + fun active_size_is_non_neg_int/1, + fun external_size_is_non_neg_int/1, + fun disk_size_is_file_size/1, + fun data_size_is_external_size/1, + fun active_size_less_than_file_size/1, + fun update_seq_is_non_neg_int/1, + fun purge_seq_is_non_neg_int/1, + fun update_opts_is_bin_list/1 ] } } }. -should_get_property({_, Info}) -> - InfoProps = [ - {signature, <<"276df562b152b3c4e5d34024f62672ed">>}, - {language, <<"javascript">>}, - {disk_size, 314}, - {data_size, 263}, - {update_seq, 11}, - {purge_seq, 0}, - {updater_running, false}, - {compact_running, false}, - {waiting_clients, 0} - ], - [ - {atom_to_list(Key), ?_assertEqual(Val, getval(Key, Info))} - || {Key, Val} <- InfoProps - ]. +sig_is_binary({_, Info}) -> + ?_assert(is_binary(prop(signature, Info))). + + +language_is_js({_, Info}) -> + ?_assertEqual(<<"javascript">>, prop(language, Info)). + + +file_size_is_non_neg_int({_, Info}) -> + ?_assert(check_non_neg_int([sizes, file], Info)). + + +active_size_is_non_neg_int({_, Info}) -> + ?_assert(check_non_neg_int([sizes, active], Info)). + + +external_size_is_non_neg_int({_, Info}) -> + ?_assert(check_non_neg_int([sizes, external], Info)). + + +disk_size_is_file_size({_, Info}) -> + ?_assertEqual(prop([sizes, file], Info), prop(disk_size, Info)). + + +data_size_is_external_size({_, Info}) -> + ?_assertEqual(prop([sizes, external], Info), prop(data_size, Info)). + + +active_size_less_than_file_size({_, Info}) -> + ?_assert(prop([sizes, active], Info) < prop([sizes, file], Info)). + + +update_seq_is_non_neg_int({_, Info}) -> + ?_assert(check_non_neg_int(update_seq, Info)). + + +purge_seq_is_non_neg_int({_, Info}) -> + ?_assert(check_non_neg_int(purge_seq, Info)). + + +update_opts_is_bin_list({_, Info}) -> + Opts = prop(update_options, Info), + ?_assert(is_list(Opts) andalso + (Opts == [] orelse lists:all([is_binary(B) || B <- Opts]))). -getval(Key, PL) -> - {value, {Key, Val}} = lists:keysearch(Key, 1, PL), - Val. +check_non_neg_int(Key, Info) -> + Size = prop(Key, Info), + is_integer(Size) andalso Size >= 0. --endif. +prop(Key, {Props}) when is_list(Props) -> + prop(Key, Props); +prop([Key], Info) -> + prop(Key, Info); +prop([Key | Rest], Info) -> + prop(Rest, prop(Key, Info)); +prop(Key, Info) when is_atom(Key), is_list(Info) -> + couch_util:get_value(Key, Info). |