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 <vatam...@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}.