You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ga...@apache.org on 2018/10/16 10:35:23 UTC

[couchdb] branch partition-review-fixes created (now 28c3b12)

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

garren pushed a change to branch partition-review-fixes
in repository https://gitbox.apache.org/repos/asf/couchdb.git.


      at 28c3b12  improvements to http layer for partitions

This branch includes the following new commits:

     new 5593e92  Mango fixes
     new fd9d87e  fix use of meck in unit tests
     new 28c3b12  improvements to http layer for partitions

The 3 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.



[couchdb] 01/03: Mango fixes

Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

garren pushed a commit to branch partition-review-fixes
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 5593e92b061c8f48eb62c1e92bf703801b887fff
Author: Garren Smith <ga...@gmail.com>
AuthorDate: Tue Oct 16 11:27:42 2018 +0200

    Mango fixes
    
    * fix issue with mango all_docs query
    * Improve partition validation check
---
 src/fabric/src/fabric_view_all_docs.erl | 24 +++++++++++++++---------
 src/mango/src/mango_idx.erl             | 13 +++++--------
 2 files changed, 20 insertions(+), 17 deletions(-)

diff --git a/src/fabric/src/fabric_view_all_docs.erl b/src/fabric/src/fabric_view_all_docs.erl
index e8868c5..635f2d8 100644
--- a/src/fabric/src/fabric_view_all_docs.erl
+++ b/src/fabric/src/fabric_view_all_docs.erl
@@ -138,8 +138,14 @@ go(DbName, _Options, Workers, QueryArgs, Callback, Acc0) ->
     end.
 
 shards(DbName, Args) ->
-    case couch_mrview_util:get_extra(Args, partitioned) of
-        true ->
+    case {couch_mrview_util:get_extra(Args, partitioned, false),
+        couch_mrview_util:get_extra(Args, partition, undefined)}  of
+        {false, _} ->
+            mem3:shards(DbName);
+        {true, undefined} ->
+            % This is an added in optimisation. If a user specifies a specific 
+            % partition in the startkey and endkey then it is possible to just choose the shards
+            % with that partition in it.
             StartKey = partition(Args#mrargs.start_key),
             EndKey = partition(Args#mrargs.end_key),
             case {StartKey, EndKey} of
@@ -148,16 +154,16 @@ shards(DbName, Args) ->
                 {_, _} ->
                     mem3:shards(DbName)
             end;
-        _ ->
-            mem3:shards(DbName)
+        {true, Partition} ->
+            mem3:shards(DbName, <<Partition/binary, ":foo">>)
     end.
 
-partition(undefined) ->
-    undefined;
-partition(null) ->
-    null;
+
 partition(Key) when is_binary(Key) ->
-    hd(binary:split(Key, <<":">>)).
+    hd(binary:split(Key, <<":">>));
+partition(_) ->
+    undefined.
+
 
 handle_message({rexi_DOWN, _, {_, NodeRef}, _}, _, State) ->
     fabric_view:check_down_shards(State, NodeRef);
diff --git a/src/mango/src/mango_idx.erl b/src/mango/src/mango_idx.erl
index 261e8f0..8c25f4a 100644
--- a/src/mango/src/mango_idx.erl
+++ b/src/mango/src/mango_idx.erl
@@ -79,12 +79,9 @@ get_usable_indexes(Db, Selector, Opts) ->
 
 filter_indexes_by_partitioned(false, Indexes, _PQ) ->
     Indexes;
-filter_indexes_by_partitioned(DbPartitioned, Indexes, PQ) ->
+filter_indexes_by_partitioned(true, Indexes, PQ) ->
     FilterFun = fun (Idx)->
-        PartitionedIdx = case lists:keyfind(partitioned, 1, Idx#idx.design_opts) of
-            {partitioned, PI} -> PI;
-            false -> DbPartitioned
-        end,
+        PartitionedIdx = couch_util:get_value(partitioned, Idx#idx.design_opts, true),
         filter_index_by_partitioned(Idx#idx.def, PartitionedIdx, PQ)
     end,
     lists:filter(FilterFun, Indexes).
@@ -148,9 +145,9 @@ validate_new(Idx, Db) ->
 
 
 validate_design_opts(Props) ->
-    case lists:keyfind(<<"options">>, 1, Props) of
-        {<<"options">>, {[{<<"partitioned">>, P}]}}
-            when is_boolean(P) ->
+    {Options} = couch_util:get_value(<<"options">>, Props, []),
+    case couch_util:get_value(<<"partitioned">>, Options) of
+        P when is_boolean(P) ->
             [{partitioned, P}];
         _ ->
             []


[couchdb] 03/03: improvements to http layer for partitions

Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

garren pushed a commit to branch partition-review-fixes
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 28c3b12ba5410ea69f2c77ace911270c334d4d6f
Author: Garren Smith <ga...@gmail.com>
AuthorDate: Tue Oct 16 12:23:46 2018 +0200

    improvements to http layer for partitions
---
 src/chttpd/src/chttpd_httpd_handlers.erl | 4 +++-
 src/chttpd/src/chttpd_view.erl           | 4 ++--
 2 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/src/chttpd/src/chttpd_httpd_handlers.erl b/src/chttpd/src/chttpd_httpd_handlers.erl
index 2659d39..e4cab7c 100644
--- a/src/chttpd/src/chttpd_httpd_handlers.erl
+++ b/src/chttpd/src/chttpd_httpd_handlers.erl
@@ -12,7 +12,7 @@
 
 -module(chttpd_httpd_handlers).
 
--export([url_handler/1, db_handler/1, design_handler/1, partition_design_handler/1]).
+-export([url_handler/1, db_handler/1, design_handler/1, partition_design_handler/1, partition_handler/1]).
 
 url_handler(<<>>)                  -> fun chttpd_misc:handle_welcome_req/1;
 url_handler(<<"favicon.ico">>)     -> fun chttpd_misc:handle_favicon_req/1;
@@ -47,3 +47,5 @@ design_handler(_) -> no_match.
 
 partition_design_handler(<<"_view">>) -> fun chttpd_view:handle_partition_view_req/4;
 partition_design_handler(_) -> no_match.
+
+partition_handler(_) -> no_match.
diff --git a/src/chttpd/src/chttpd_view.erl b/src/chttpd/src/chttpd_view.erl
index 7c1dc81..c1dc186 100644
--- a/src/chttpd/src/chttpd_view.erl
+++ b/src/chttpd/src/chttpd_view.erl
@@ -151,8 +151,8 @@ check_partition_restrictions(#mrargs{} = Args) ->
         {<<"stable">>, Args#mrargs.stable, true},
         {<<"conflicts">>, Args#mrargs.conflicts, true}
     ],
-    lists:foreach(fun ({Param, Field, Value}) ->
-        case Field =:= Value of
+    lists:foreach(fun ({Param, ArgValue, RestrictedValue}) ->
+        case ArgValue =:= RestrictedValue of
             true ->
                 Msg = [<<"`">>, Param, <<"=true` is not supported in this view.">>],
                 throw({bad_request, ?l2b(Msg)});


[couchdb] 02/03: fix use of meck in unit tests

Posted by ga...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

garren pushed a commit to branch partition-review-fixes
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit fd9d87ed9c984234625214b47d366078a0165ad7
Author: Garren Smith <ga...@gmail.com>
AuthorDate: Tue Oct 16 12:23:19 2018 +0200

    fix use of meck in unit tests
---
 src/couch/test/couchdb_views_tests.erl  |   1 -
 src/fabric/src/fabric_view_all_docs.erl | 177 +++++++++++++++++++-------------
 src/mem3/src/mem3.erl                   |  26 +++--
 3 files changed, 122 insertions(+), 82 deletions(-)

diff --git a/src/couch/test/couchdb_views_tests.erl b/src/couch/test/couchdb_views_tests.erl
index a023a8c..1b1a8e5 100644
--- a/src/couch/test/couchdb_views_tests.erl
+++ b/src/couch/test/couchdb_views_tests.erl
@@ -60,7 +60,6 @@ setup_legacy() ->
     lists:foreach(fun(File) -> file:delete(File) end, Files),
 
     % copy old db file into db dir
-    ?debugFmt("copy ~p ~p", [OldDbFilePath, NewDbFilePath]),
     {ok, _} = file:copy(OldDbFilePath, NewDbFilePath),
 
     % copy old view file into view dir
diff --git a/src/fabric/src/fabric_view_all_docs.erl b/src/fabric/src/fabric_view_all_docs.erl
index 635f2d8..6eb626a 100644
--- a/src/fabric/src/fabric_view_all_docs.erl
+++ b/src/fabric/src/fabric_view_all_docs.erl
@@ -327,85 +327,114 @@ cancel_read_pids(Pids) ->
 -ifdef(TEST).
 
 -include_lib("eunit/include/eunit.hrl").
+    check_partitions_work_with_shards_test_() ->
+        {
+            foreach,
+            fun setup/0,
+            fun teardown/1,
+            [
+                t_shards_for_partition_gets_partitioned_shards(),
+                t_shards_for_no_partition_gets_all_shards(),
+                t_shards_for_different_partitions_gets_all_shards(),
+                t_shards_for_no_startkey_all_shards(),
+                t_shards_for_no_endkey_all_shards(),
+                t_shards_for_no_keys_all_shards(),
+                t_shards_for_non_binary_keys_all_shards()
 
-    shards_for_partition_gets_partitioned_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            start_key = <<"pk:id">>,
-            end_key = <<"pk:idZ">>,
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>, <<"pk:foo">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
-
-    shards_for_no_partition_gets_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            start_key = <<"pk:id">>,
-            end_key = <<"pk:idZ">>,
-            extra = [{partitioned, false}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+            ]
+        }.
+
+    setup() ->
+        ok.
+
+    teardown(_) ->
+        meck:unload().
+
+    t_shards_for_partition_gets_partitioned_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                start_key = <<"pk:id">>,
+                end_key = <<"pk:idZ">>,
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>, <<"pk:foo">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
+
+    t_shards_for_no_partition_gets_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                start_key = <<"pk:id">>,
+                end_key = <<"pk:idZ">>,
+                extra = [{partitioned, false}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
     
-    shards_for_different_partitions_gets_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            start_key = <<"pk1:id">>,
-            end_key = <<"pk2:idZ">>,
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+    t_shards_for_different_partitions_gets_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                start_key = <<"pk1:id">>,
+                end_key = <<"pk2:idZ">>,
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
     
-    shards_for_no_startkey_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            end_key = <<"pk:idZ">>,
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+    t_shards_for_no_startkey_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                end_key = <<"pk:idZ">>,
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
     
-    shards_for_no_endkey_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            start_key = <<"pk:idZ">>,
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+    t_shards_for_no_endkey_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                start_key = <<"pk:idZ">>,
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
     
-    shards_for_no_keys_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+    t_shards_for_no_keys_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
     
-    shards_for_non_binary_keys_all_shards_test() ->
-        DbName = <<"db">>,
-        Args = #mrargs{
-            start_key = null,
-            end_key = null,
-            extra = [{partitioned, true}]
-        },
-        meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
-        shards(DbName, Args),
-        meck:validate(mem3),
-        meck:unload(mem3).
+    t_shards_for_non_binary_keys_all_shards() ->
+        ?_test(begin
+            DbName = <<"db">>,
+            Args = #mrargs{
+                start_key = null,
+                end_key = null,
+                extra = [{partitioned, true}]
+            },
+            meck:expect(mem3, shards, fun(<<"db">>) -> [] end),
+            shards(DbName, Args),
+            ?assertEqual(1, meck:num_calls(mem3, shards, '_'))
+        end).
 
 -endif.
diff --git a/src/mem3/src/mem3.erl b/src/mem3/src/mem3.erl
index d6d14dd..17a2322 100644
--- a/src/mem3/src/mem3.erl
+++ b/src/mem3/src/mem3.erl
@@ -412,15 +412,27 @@ allowed_nodes_test_() ->
         ]
     }]}.
 
-is_partitioned_false_shards_db_test() ->
-    meck:expect(config, get, fun (_, _, Default) -> Default end),
-    ?assertEqual(is_partitioned(<<"_dbs">>), false),
+check_partitions_and_system_dbs_test_() ->
+        {
+            foreach,
+            fun setup/0,
+            fun teardown/1,
+            [
+                t_is_partitioned_false_shards_db()
+            ]
+        }.
+
+setup() ->
+        ok.
+
+teardown(_) ->
     meck:unload().
 
-is_partitioned_false_nodes_db_test() ->
-    meck:expect(config, get, fun (_, _, Default) -> Default end),
-    ?assertEqual(is_partitioned(<<"_nodes">>), false),
-    meck:unload().
+t_is_partitioned_false_shards_db() ->
+    ?_test(begin
+        meck:expect(config, get, fun (_, _, Default) -> Default end),
+        ?assertEqual(is_partitioned(<<"_dbs">>), false)
+    end).
 
 is_partitioned_true_partitioned_db_test() ->
     Shard = #shard{