diff options
author | Nick Vatamaniuc <vatamane@apache.org> | 2018-07-11 13:35:25 -0400 |
---|---|---|
committer | Nick Vatamaniuc <nickva@users.noreply.github.com> | 2018-07-11 14:46:42 -0400 |
commit | bfc610f3b9bbe087f4d00a570bb9cde3df78a35f (patch) | |
tree | 03575956bca03f33948f448cb9e7af29ad612029 | |
parent | 7dfed0cbbd2695835698efca438e6b8d9fa17f88 (diff) | |
download | couchdb-bfc610f3b9bbe087f4d00a570bb9cde3df78a35f.tar.gz |
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
-rw-r--r-- | src/couch/src/couch_db_updater.erl | 13 | ||||
-rw-r--r-- | src/couch/src/couch_key_tree.erl | 13 | ||||
-rw-r--r-- | src/couch/src/test_engine_util.erl | 3 | ||||
-rw-r--r-- | 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 fba99a773..acb9ec1c9 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 5d5361507..94150418e 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 89997538d..fef9e9f92 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 2b7d5fe62..5d9cc8372 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}. |