You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by ji...@apache.org on 2018/06/25 12:40:18 UTC

[couchdb] 01/01: Address Ilya and Paul's comments

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

jiangphcn pushed a commit to branch COUCHDB-3326-clustered-purge-pr5-implementation
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 63428aeffc1daa6487d869da86dfc264cc9f1ec8
Author: jiangphcn <ji...@cn.ibm.com>
AuthorDate: Mon Jun 25 19:21:07 2018 +0800

    Address Ilya and Paul's comments
    
       - pass DbName to is_valid_purge_client/2
       - remove duplicate "Limit = chttpd:json_body(Req)"
       - remove ok tag when calling get_oldest_purge_seq/1
    COUCHDB-3326
---
 src/chttpd/src/chttpd_db.erl                                  |  1 -
 src/couch/src/couch_db.erl                                    |  2 +-
 src/couch/src/couch_db_plugin.erl                             |  6 +++---
 src/couch_index/src/couch_index_plugin_couch_db.erl           |  6 +++---
 src/couch_mrview/src/couch_mrview_index.erl                   |  6 ++----
 .../test/couch_mrview_purge_docs_fabric_tests.erl             |  8 ++++++--
 src/mem3/src/mem3_plugin_couch_db.erl                         |  6 +++---
 src/mem3/src/mem3_rep.erl                                     | 11 +++++------
 8 files changed, 23 insertions(+), 23 deletions(-)

diff --git a/src/chttpd/src/chttpd_db.erl b/src/chttpd/src/chttpd_db.erl
index b9652bc..83375cd 100644
--- a/src/chttpd/src/chttpd_db.erl
+++ b/src/chttpd/src/chttpd_db.erl
@@ -620,7 +620,6 @@ db_req(#httpd{path_parts=[_,<<"_revs_limit">>]}=Req, _Db) ->
     send_method_not_allowed(Req, "PUT,GET");
 
 db_req(#httpd{method='PUT',path_parts=[_,<<"_purged_infos_limit">>]}=Req, Db) ->
-    Limit = chttpd:json_body(Req),
     Options = [{user_ctx, Req#httpd.user_ctx}],
     case chttpd:json_body(Req) of
         Limit when is_integer(Limit), Limit > 0 ->
diff --git a/src/couch/src/couch_db.erl b/src/couch/src/couch_db.erl
index 9d9d164..b3f0269 100644
--- a/src/couch/src/couch_db.erl
+++ b/src/couch/src/couch_db.erl
@@ -474,7 +474,7 @@ purge_client_exists(DbName, DocId, Props) ->
     LagThreshold = NowSecs - LagWindow,
 
     try
-        Exists = couch_db_plugin:is_valid_purge_client(Props),
+        Exists = couch_db_plugin:is_valid_purge_client(DbName, Props),
         if not Exists -> ok; true ->
             Updated = couch_util:get_value(<<"updated_on">>, Props),
             if is_integer(Updated) and Updated > LagThreshold -> ok; true ->
diff --git a/src/couch/src/couch_db_plugin.erl b/src/couch/src/couch_db_plugin.erl
index c0bcc2f..e25866e 100644
--- a/src/couch/src/couch_db_plugin.erl
+++ b/src/couch/src/couch_db_plugin.erl
@@ -18,7 +18,7 @@
     after_doc_read/2,
     validate_docid/1,
     check_is_admin/1,
-    is_valid_purge_client/1,
+    is_valid_purge_client/2,
     on_compact/2,
     on_delete/2
 ]).
@@ -58,10 +58,10 @@ check_is_admin(Db) ->
     %% callbacks return true only if it specifically allow the given Id
     couch_epi:any(Handle, ?SERVICE_ID, check_is_admin, [Db], []).
 
-is_valid_purge_client(Props) ->
+is_valid_purge_client(DbName, Props) ->
     Handle = couch_epi:get_handle(?SERVICE_ID),
     %% callbacks return true only if it specifically allow the given Id
-    couch_epi:any(Handle, ?SERVICE_ID, is_valid_purge_client, [Props], []).
+    couch_epi:any(Handle, ?SERVICE_ID, is_valid_purge_client, [DbName, Props], []).
 
 on_compact(DbName, DDocs) ->
     Handle = couch_epi:get_handle(?SERVICE_ID),
diff --git a/src/couch_index/src/couch_index_plugin_couch_db.erl b/src/couch_index/src/couch_index_plugin_couch_db.erl
index 5d4a6ac..0af22e3 100644
--- a/src/couch_index/src/couch_index_plugin_couch_db.erl
+++ b/src/couch_index/src/couch_index_plugin_couch_db.erl
@@ -13,13 +13,13 @@
 -module(couch_index_plugin_couch_db).
 
 -export([
-    is_valid_purge_client/1,
+    is_valid_purge_client/2,
     on_compact/2
 ]).
 
 
-is_valid_purge_client(Props) ->
-    couch_mrview_index:verify_index_exists(Props).
+is_valid_purge_client(DbName, Props) ->
+    couch_mrview_index:verify_index_exists(DbName, Props).
 
 
 on_compact(DbName, DDocs) ->
diff --git a/src/couch_mrview/src/couch_mrview_index.erl b/src/couch_mrview/src/couch_mrview_index.erl
index d6558e1..caf4f0c 100644
--- a/src/couch_mrview/src/couch_mrview_index.erl
+++ b/src/couch_mrview/src/couch_mrview_index.erl
@@ -18,7 +18,7 @@
 -export([start_update/4, purge/4, process_doc/3, finish_update/1, commit/1]).
 -export([compact/3, swap_compacted/2, remove_compacted/1]).
 -export([index_file_exists/1]).
--export([update_local_purge_doc/2, verify_index_exists/1]).
+-export([update_local_purge_doc/2, verify_index_exists/2]).
 -export([ensure_local_purge_docs/2]).
 
 -include_lib("couch/include/couch_db.hrl").
@@ -219,11 +219,10 @@ index_file_exists(State) ->
     filelib:is_file(IndexFName).
 
 
-verify_index_exists(Props) ->
+verify_index_exists(DbName, Props) ->
     try
         Type = couch_util:get_value(<<"type">>, Props),
         if Type =/= <<"mrview">> -> false; true ->
-            DbName = couch_util:get_value(<<"dbname">>, Props),
             DDocId = couch_util:get_value(<<"ddoc_id">>, Props),
             couch_util:with_db(DbName, fun(Db) ->
                 {ok, DesignDocs} = couch_db:get_design_docs(Db),
@@ -307,7 +306,6 @@ update_local_purge_doc(Db, State, PSeq) ->
         {<<"type">>, <<"mrview">>},
         {<<"purge_seq">>, PSeq},
         {<<"updated_on">>, NowSecs},
-        {<<"dbname">>, get(db_name, State)},
         {<<"ddoc_id">>, get(idx_name, State)},
         {<<"signature">>, Sig}
     ]}),
diff --git a/src/couch_mrview/test/couch_mrview_purge_docs_fabric_tests.erl b/src/couch_mrview/test/couch_mrview_purge_docs_fabric_tests.erl
index 6b0d4d9..213acac 100644
--- a/src/couch_mrview/test/couch_mrview_purge_docs_fabric_tests.erl
+++ b/src/couch_mrview/test/couch_mrview_purge_docs_fabric_tests.erl
@@ -78,7 +78,10 @@ test_purge_verify_index(DbName) ->
 
         {ok, #doc{body = {Props1}}} = get_local_purge_doc(DbName),
         ?assertEqual(0, couch_util:get_value(<<"purge_seq">>, Props1)),
-        ?assertEqual(true, couch_mrview_index:verify_index_exists(Props1)),
+        ShardNames = [Sh || #shard{name = Sh} <- mem3:local_shards(DbName)],
+        [ShardDbName | _Rest ] = ShardNames,
+        ?assertEqual(true, couch_mrview_index:verify_index_exists(
+            ShardDbName, Props1)),
 
         purge_docs(DbName, [<<"1">>]),
 
@@ -94,7 +97,8 @@ test_purge_verify_index(DbName) ->
 
         {ok, #doc{body = {Props2}}} = get_local_purge_doc(DbName),
         ?assertEqual(1, couch_util:get_value(<<"purge_seq">>, Props2)),
-        ?assertEqual(true, couch_mrview_index:verify_index_exists(Props2))
+        ?assertEqual(true, couch_mrview_index:verify_index_exists(
+            ShardDbName, Props2))
     end).
 
 
diff --git a/src/mem3/src/mem3_plugin_couch_db.erl b/src/mem3/src/mem3_plugin_couch_db.erl
index f19f5eb..8cb5d78 100644
--- a/src/mem3/src/mem3_plugin_couch_db.erl
+++ b/src/mem3/src/mem3_plugin_couch_db.erl
@@ -13,9 +13,9 @@
 -module(mem3_plugin_couch_db).
 
 -export([
-    is_valid_purge_client/1
+    is_valid_purge_client/2
 ]).
 
 
-is_valid_purge_client(Props) ->
-    mem3_rep:verify_purge_checkpoint(Props).
+is_valid_purge_client(DbName, Props) ->
+    mem3_rep:verify_purge_checkpoint(DbName, Props).
diff --git a/src/mem3/src/mem3_rep.erl b/src/mem3/src/mem3_rep.erl
index 22a3f7a..3a4147e 100644
--- a/src/mem3/src/mem3_rep.erl
+++ b/src/mem3/src/mem3_rep.erl
@@ -18,7 +18,7 @@
     go/3,
     make_local_id/2,
     make_purge_id/2,
-    verify_purge_checkpoint/1,
+    verify_purge_checkpoint/2,
     find_source_seq/4
 ]).
 
@@ -125,11 +125,10 @@ make_purge_id(SourceUUID, TargetUUID) ->
     <<"_local/purge-mem3-", SourceUUID/binary, "-", TargetUUID/binary>>.
 
 
-verify_purge_checkpoint(Props) ->
+verify_purge_checkpoint(DbName, Props) ->
     try
         Type = couch_util:get_value(<<"type">>, Props),
         if Type =/= <<"internal_replication">> -> false; true ->
-            DbName = couch_util:get_value(<<"dbname">>, Props),
             SourceBin = couch_util:get_value(<<"source">>, Props),
             TargetBin = couch_util:get_value(<<"target">>, Props),
             Range = couch_util:get_value(<<"range">>, Props),
@@ -249,8 +248,9 @@ pull_purges(#acc{} = Acc0) ->
         end,
 
         if Remaining =< 0 -> ok; true ->
-            OldestPurgeSeq = couch_db:get_oldest_purge_seq(Db),
-            PurgesToPush = couch_db:get_purge_seq(Db) - OldestPurgeSeq,
+            {ok, PurgeSeq} = couch_db:get_purge_seq(Db),
+            {ok, OldestPurgeSeq} = couch_db:get_oldest_purge_seq(Db),
+            PurgesToPush = PurgeSeq - OldestPurgeSeq,
             Changes = couch_db:count_changes_since(Db, UpdateSeq),
             throw({finished, Remaining + PurgesToPush + Changes})
         end,
@@ -505,7 +505,6 @@ purge_cp_body(#acc{} = Acc, PurgeSeq) ->
         {<<"type">>, <<"internal_replication">>},
         {<<"updated_on">>, NowSecs},
         {<<"purge_seq">>, PurgeSeq},
-        {<<"dbname">>, Source#shard.dbname},
         {<<"source">>, atom_to_binary(Source#shard.node, latin1)},
         {<<"target">>, atom_to_binary(Target#shard.node, latin1)},
         {<<"range">>, Source#shard.range}