You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by da...@apache.org on 2018/06/15 18:39:36 UTC

[couchdb] branch master updated: Fix active size calculations for views

This is an automated email from the ASF dual-hosted git repository.

davisp pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/couchdb.git


The following commit(s) were added to refs/heads/master by this push:
     new 000766c  Fix active size calculations for views
000766c is described below

commit 000766c4f8b51562757818dfa36b3e6579d16309
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Fri Jun 15 11:44:20 2018 -0500

    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.
---
 src/couch_mrview/src/couch_mrview_index.erl        |  3 +-
 src/couch_mrview/src/couch_mrview_util.erl         | 17 ++++
 .../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 aa1ee27..5d285d6 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 0c6e5fc..eb461d0 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 c994df9..efa03e7 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).

-- 
To stop receiving notification emails like this one, please contact
davisp@apache.org.