summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorPaul J. Davis <paul.joseph.davis@gmail.com>2018-06-15 11:44:20 -0500
committerPaul J. Davis <paul.joseph.davis@gmail.com>2018-06-15 11:44:20 -0500
commit1cf5e343a68a3b540e0cf9f9b2f851a64d3583b6 (patch)
treee9ae5d29069fbd958c0a694b9290e502db410733
parentc7d35c3133499fcb2f7989aea0c4c0e9277351f9 (diff)
downloadcouchdb-fix-couch-mrview-active-size-calculation.tar.gz
Fix active size calculations for viewsfix-couch-mrview-active-size-calculation
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.erl3
-rw-r--r--src/couch_mrview/src/couch_mrview_util.erl17
-rw-r--r--src/couch_mrview/test/couch_mrview_index_info_tests.erl96
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).