couchdb-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From vatam...@apache.org
Subject [couchdb] branch master updated: Make stem_interactive_updates option work again
Date Wed, 11 Jul 2018 18:46:45 GMT
This is an automated email from the ASF dual-hosted git repository.

vatamane 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 bfc610f  Make stem_interactive_updates option work again
bfc610f is described below

commit bfc610f3b9bbe087f4d00a570bb9cde3df78a35f
Author: Nick Vatamaniuc <vatamane@apache.org>
AuthorDate: Wed Jul 11 13:35:25 2018 -0400

    Make stem_interactive_updates option work again
    
    After the aebdbc452573f70f4e50d88af5814d0fbe936333 stemming is done separately
    from merge so stem interactive option didn't take effect. That is mostly ok as
    speed improvements should reduce the need for that option, but it still might
    be nice to keep the option (just in case).
    
    Also, a nice side effect is it removes an extra external function from
    couch_key_tree module and simplifies the tests a bit.
    
    Related PR: #958
---
 src/couch/src/couch_db_updater.erl      |  13 ++-
 src/couch/src/couch_key_tree.erl        |  13 ---
 src/couch/src/test_engine_util.erl      |   3 +-
 src/couch/test/couch_key_tree_tests.erl | 177 ++++++++++++++------------------
 4 files changed, 87 insertions(+), 119 deletions(-)

diff --git a/src/couch/src/couch_db_updater.erl b/src/couch/src/couch_db_updater.erl
index fba99a7..acb9ec1 100644
--- a/src/couch/src/couch_db_updater.erl
+++ b/src/couch/src/couch_db_updater.erl
@@ -506,7 +506,7 @@ merge_rev_trees(Limit, MergeConflicts, [NewDocs|RestDocsList],
     NewDocInfo0 = lists:foldl(fun({Client, NewDoc}, OldInfoAcc) ->
         merge_rev_tree(OldInfoAcc, NewDoc, Client, MergeConflicts)
     end, OldDocInfo, NewDocs),
-    NewDocInfo1 = stem_full_doc_info(NewDocInfo0, Limit),
+    NewDocInfo1 = maybe_stem_full_doc_info(NewDocInfo0, Limit),
     % When MergeConflicts is false, we updated #full_doc_info.deleted on every
     % iteration of merge_rev_tree. However, merge_rev_tree does not update
     % #full_doc_info.deleted when MergeConflicts is true, since we don't need
@@ -624,9 +624,14 @@ merge_rev_tree(OldInfo, NewDoc, _Client, true) ->
     {NewTree, _} = couch_key_tree:merge(OldTree, NewTree0),
     OldInfo#full_doc_info{rev_tree = NewTree}.
 
-stem_full_doc_info(#full_doc_info{rev_tree = Tree} = Info, Limit) ->
-    Stemmed = couch_key_tree:stem(Tree, Limit),
-    Info#full_doc_info{rev_tree = Stemmed}.
+maybe_stem_full_doc_info(#full_doc_info{rev_tree = Tree} = Info, Limit) ->
+    case config:get_boolean("couchdb", "stem_interactive_updates", true) of
+        true ->
+            Stemmed = couch_key_tree:stem(Tree, Limit),
+            Info#full_doc_info{rev_tree = Stemmed};
+        false ->
+            Info
+    end.
 
 update_docs_int(Db, DocsList, LocalDocs, MergeConflicts, FullCommit) ->
     UpdateSeq = couch_db_engine:get_update_seq(Db),
diff --git a/src/couch/src/couch_key_tree.erl b/src/couch/src/couch_key_tree.erl
index 5d53615..9415041 100644
--- a/src/couch/src/couch_key_tree.erl
+++ b/src/couch/src/couch_key_tree.erl
@@ -60,7 +60,6 @@ map/2,
 map_leafs/2,
 mapfold/3,
 multi_merge/2,
-merge/3,
 merge/2,
 remove_leafs/2,
 stem/2
@@ -81,18 +80,6 @@ multi_merge(RevTree, Trees) ->
     end, RevTree, lists:sort(Trees)).
 
 
-%% @doc Merge a path into the given tree and then stem the result.
-%% Although Tree is of type tree(), it must not contain any branches.
--spec merge(revtree(), tree() | path(), pos_integer()) ->
-                {revtree(), new_leaf | new_branch | internal_node}.
-merge(RevTree, Tree, StemDepth) ->
-    {Merged, Result} = merge(RevTree, Tree),
-    case config:get("couchdb", "stem_interactive_updates", "true") of
-        "true" -> {stem(Merged, StemDepth), Result};
-        _ -> {Merged, Result}
-    end.
-
-
 %% @doc Merge a path into a tree.
 -spec merge(revtree(), tree() | path()) ->
                 {revtree(), new_leaf | new_branch | internal_node}.
diff --git a/src/couch/src/test_engine_util.erl b/src/couch/src/test_engine_util.erl
index 8999753..fef9e9f 100644
--- a/src/couch/src/test_engine_util.erl
+++ b/src/couch/src/test_engine_util.erl
@@ -309,7 +309,8 @@ gen_write(Engine, St, {Action, {DocId, Body, Atts0}}, UpdateSeq) ->
         conflict -> new_branch;
         _ -> new_leaf
     end,
-    {NewTree, NodeType} = couch_key_tree:merge(PrevRevTree, Path, RevsLimit),
+    {MergedTree, NodeType} = couch_key_tree:merge(PrevRevTree, Path),
+    NewTree = couch_key_tree:stem(MergedTree, RevsLimit),
 
     NewFDI = PrevFDI#full_doc_info{
         deleted = couch_doc:is_deleted(NewTree),
diff --git a/src/couch/test/couch_key_tree_tests.erl b/src/couch/test/couch_key_tree_tests.erl
index 2b7d5fe..5d9cc83 100644
--- a/src/couch/test/couch_key_tree_tests.erl
+++ b/src/couch/test/couch_key_tree_tests.erl
@@ -16,138 +16,108 @@
 
 -define(DEPTH, 10).
 
-setup() ->
-    meck:new(config),
-    meck:expect(config, get, fun(_, _, Default) -> Default end).
-
-teardown(_) ->
-    meck:unload(config).
 
 key_tree_merge_test_()->
     {
         "Key tree merge",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_merge_with_empty_tree(),
-                should_merge_reflexive(),
-                should_merge_prefix_of_a_tree_with_tree(),
-                should_produce_conflict_on_merge_with_unrelated_branch(),
-                should_merge_reflexive_for_child_nodes(),
-                should_merge_tree_to_itself(),
-                should_merge_tree_of_odd_length(),
-                should_merge_tree_with_stem(),
-                should_merge_with_stem_at_deeper_level(),
-                should_merge_with_stem_at_deeper_level_with_deeper_paths(),
-                should_merge_single_tree_with_deeper_stem(),
-                should_merge_tree_with_large_stem(),
-                should_merge_stems(),
-                should_create_conflicts_on_merge(),
-                should_create_no_conflicts_on_merge(),
-                should_ignore_conflicting_branch()
-            ]
-        }
+        [
+            should_merge_with_empty_tree(),
+            should_merge_reflexive(),
+            should_merge_prefix_of_a_tree_with_tree(),
+            should_produce_conflict_on_merge_with_unrelated_branch(),
+            should_merge_reflexive_for_child_nodes(),
+            should_merge_tree_to_itself(),
+            should_merge_tree_of_odd_length(),
+            should_merge_tree_with_stem(),
+            should_merge_with_stem_at_deeper_level(),
+            should_merge_with_stem_at_deeper_level_with_deeper_paths(),
+            should_merge_single_tree_with_deeper_stem(),
+            should_merge_tree_with_large_stem(),
+            should_merge_stems(),
+            should_create_conflicts_on_merge(),
+            should_create_no_conflicts_on_merge(),
+            should_ignore_conflicting_branch()
+        ]
     }.
 
 key_tree_missing_leaves_test_()->
     {
-        "Missing tree leaves",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_not_find_missing_leaves(),
-                should_find_missing_leaves()
-            ]
-        }
+         "Missing tree leaves",
+         [
+             should_not_find_missing_leaves(),
+             should_find_missing_leaves()
+         ]
     }.
 
 key_tree_remove_leaves_test_()->
     {
         "Remove tree leaves",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_effect_on_removing_no_leaves(),
-                should_have_no_effect_on_removing_non_existant_branch(),
-                should_remove_leaf(),
-                should_produce_empty_tree_on_removing_all_leaves(),
-                should_have_no_effect_on_removing_non_existant_node(),
-                should_produce_empty_tree_on_removing_last_leaf()
-            ]
-        }
+        [
+            should_have_no_effect_on_removing_no_leaves(),
+            should_have_no_effect_on_removing_non_existant_branch(),
+            should_remove_leaf(),
+            should_produce_empty_tree_on_removing_all_leaves(),
+            should_have_no_effect_on_removing_non_existant_node(),
+            should_produce_empty_tree_on_removing_last_leaf()
+        ]
     }.
 
 key_tree_get_leaves_test_()->
     {
         "Leaves retrieving",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_extract_subtree(),
-                should_extract_subsubtree(),
-                should_gather_non_existant_leaf(),
-                should_gather_leaf(),
-                shoul_gather_multiple_leaves(),
-                should_gather_single_leaf_for_multiple_revs(),
-                should_gather_multiple_for_multiple_revs(),
-                should_retrieve_full_key_path(),
-                should_retrieve_full_key_path_for_node(),
-                should_retrieve_leaves_with_parent_node(),
-                should_retrieve_all_leaves()
-            ]
-        }
+        [
+            should_extract_subtree(),
+            should_extract_subsubtree(),
+            should_gather_non_existant_leaf(),
+            should_gather_leaf(),
+            shoul_gather_multiple_leaves(),
+            should_gather_single_leaf_for_multiple_revs(),
+            should_gather_multiple_for_multiple_revs(),
+            should_retrieve_full_key_path(),
+            should_retrieve_full_key_path_for_node(),
+            should_retrieve_leaves_with_parent_node(),
+            should_retrieve_all_leaves()
+        ]
     }.
 
 key_tree_leaf_counting_test_()->
     {
         "Leaf counting",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_leaves_for_empty_tree(),
-                should_have_single_leaf_for_tree_with_single_node(),
-                should_have_two_leaves_for_tree_with_chindler_siblings(),
-                should_not_affect_on_leaf_counting_for_stemmed_tree()
-            ]
-        }
+        [
+            should_have_no_leaves_for_empty_tree(),
+            should_have_single_leaf_for_tree_with_single_node(),
+            should_have_two_leaves_for_tree_with_chindler_siblings(),
+            should_not_affect_on_leaf_counting_for_stemmed_tree()
+        ]
     }.
 
 key_tree_stemming_test_()->
     {
         "Stemming",
-        {
-            setup,
-            fun setup/0, fun teardown/1,
-            [
-                should_have_no_effect_for_stemming_more_levels_than_exists(),
-                should_return_one_deepest_node(),
-                should_return_two_deepest_nodes()
-            ]
-        }
+        [
+            should_have_no_effect_for_stemming_more_levels_than_exists(),
+            should_return_one_deepest_node(),
+            should_return_two_deepest_nodes()
+        ]
     }.
 
 
 should_merge_with_empty_tree()->
     One = {1, {"1","foo",[]}},
     ?_assertEqual({[One], new_leaf},
-                  couch_key_tree:merge([], One, ?DEPTH)).
+                  merge_and_stem([], One)).
 
 should_merge_reflexive()->
     One = {1, {"1","foo",[]}},
     ?_assertEqual({[One], internal_node},
-                  couch_key_tree:merge([One], One, ?DEPTH)).
+                  merge_and_stem([One], One)).
 
 should_merge_prefix_of_a_tree_with_tree()->
     One = {1, {"1","foo",[]}},
     TwoSibs = [{1, {"1","foo",[]}},
                {1, {"2","foo",[]}}],
     ?_assertEqual({TwoSibs, internal_node},
-                  couch_key_tree:merge(TwoSibs, One, ?DEPTH)).
+                  merge_and_stem(TwoSibs, One)).
 
 should_produce_conflict_on_merge_with_unrelated_branch()->
     TwoSibs = [{1, {"1","foo",[]}},
@@ -157,12 +127,12 @@ should_produce_conflict_on_merge_with_unrelated_branch()->
                  {1, {"2","foo",[]}},
                  {1, {"3","foo",[]}}],
     ?_assertEqual({ThreeSibs, new_branch},
-                  couch_key_tree:merge(TwoSibs, Three, ?DEPTH)).
+                  merge_and_stem(TwoSibs, Three)).
 
 should_merge_reflexive_for_child_nodes()->
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], TwoChild, ?DEPTH)).
+                  merge_and_stem([TwoChild], TwoChild)).
 
 should_merge_tree_to_itself()->
     TwoChildSibs = {1, {"1","foo", [{"1a", "bar", []},
@@ -170,7 +140,7 @@ should_merge_tree_to_itself()->
     Leafs = couch_key_tree:get_all_leafs([TwoChildSibs]),
     Paths = lists:map(fun leaf_to_path/1, Leafs),
     FinalTree = lists:foldl(fun(Path, TreeAcc) ->
-        {NewTree, internal_node} = couch_key_tree:merge(TreeAcc, Path),
+        {NewTree, internal_node} = merge_and_stem(TreeAcc, Path),
         NewTree
     end, [TwoChildSibs], Paths),
     ?_assertEqual([TwoChildSibs], FinalTree).
@@ -192,7 +162,7 @@ should_merge_tree_of_odd_length()->
     TwoChildPlusSibs = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]},
                                         {"1b", "bar", []}]}},
     ?_assertEqual({[TwoChildPlusSibs], new_leaf},
-                  couch_key_tree:merge([TwoChildSibs], TwoChild, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], TwoChild)).
 
 should_merge_tree_with_stem()->
     Stemmed = {2, {"1a", "bar", []}},
@@ -200,52 +170,52 @@ should_merge_tree_with_stem()->
                                     {"1b", "bar", []}]}},
 
     ?_assertEqual({[TwoChildSibs], internal_node},
-                  couch_key_tree:merge([TwoChildSibs], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], Stemmed)).
 
 should_merge_with_stem_at_deeper_level()->
     Stemmed = {3, {"1bb", "boo", []}},
     TwoChildSibs = {1, {"1","foo", [{"1a", "bar", []},
                                     {"1b", "bar", [{"1bb", "boo", []}]}]}},
     ?_assertEqual({[TwoChildSibs], internal_node},
-                  couch_key_tree:merge([TwoChildSibs], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChildSibs], Stemmed)).
 
 should_merge_with_stem_at_deeper_level_with_deeper_paths()->
     Stemmed = {3, {"1bb", "boo", []}},
     StemmedTwoChildSibs = [{2,{"1a", "bar", []}},
                            {2,{"1b", "bar", [{"1bb", "boo", []}]}}],
     ?_assertEqual({StemmedTwoChildSibs, internal_node},
-                  couch_key_tree:merge(StemmedTwoChildSibs, Stemmed, ?DEPTH)).
+                  merge_and_stem(StemmedTwoChildSibs, Stemmed)).
 
 should_merge_single_tree_with_deeper_stem()->
     Stemmed = {3, {"1aa", "bar", []}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChild], Stemmed)).
 
 should_merge_tree_with_large_stem()->
     Stemmed = {2, {"1a", "bar", [{"1aa", "bar", []}]}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], internal_node},
-                  couch_key_tree:merge([TwoChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([TwoChild], Stemmed)).
 
 should_merge_stems()->
     StemmedA = {2, {"1a", "bar", [{"1aa", "bar", []}]}},
     StemmedB = {3, {"1aa", "bar", []}},
     ?_assertEqual({[StemmedA], internal_node},
-                  couch_key_tree:merge([StemmedA], StemmedB, ?DEPTH)).
+                  merge_and_stem([StemmedA], StemmedB)).
 
 should_create_conflicts_on_merge()->
     OneChild = {1, {"1","foo",[{"1a", "bar", []}]}},
     Stemmed = {3, {"1aa", "bar", []}},
     ?_assertEqual({[OneChild, Stemmed], new_branch},
-                  couch_key_tree:merge([OneChild], Stemmed, ?DEPTH)).
+                  merge_and_stem([OneChild], Stemmed)).
 
 should_create_no_conflicts_on_merge()->
     OneChild = {1, {"1","foo",[{"1a", "bar", []}]}},
     Stemmed = {3, {"1aa", "bar", []}},
     TwoChild = {1, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}},
     ?_assertEqual({[TwoChild], new_leaf},
-                  couch_key_tree:merge([OneChild, Stemmed], TwoChild, ?DEPTH)).
+                  merge_and_stem([OneChild, Stemmed], TwoChild)).
 
 should_ignore_conflicting_branch()->
     %% this test is based on couch-902-test-case2.py
@@ -274,7 +244,7 @@ should_ignore_conflicting_branch()->
     {
         "COUCHDB-902",
         ?_assertEqual({[FooBar], new_leaf},
-                      couch_key_tree:merge([Foo], Bar, ?DEPTH))
+                      merge_and_stem([Foo], Bar))
     }.
 
 should_not_find_missing_leaves()->
@@ -436,3 +406,8 @@ should_return_two_deepest_nodes()->
     TwoChild = [{0, {"1","foo", [{"1a", "bar", [{"1aa", "bar", []}]}]}}],
     Stemmed = [{1, {"1a", "bar", [{"1aa", "bar", []}]}}],
     ?_assertEqual(Stemmed, couch_key_tree:stem(TwoChild, 2)).
+
+
+merge_and_stem(RevTree, Tree) ->
+    {Merged, Result} = couch_key_tree:merge(RevTree, Tree),
+    {couch_key_tree:stem(Merged, ?DEPTH), Result}.


Mime
View raw message