You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by va...@apache.org on 2018/07/11 18:46:45 UTC
[couchdb] branch master updated: Make stem_interactive_updates
option work again
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 <va...@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}.