[couchdb] branch master updated: Refactor fabric:cleanup_index_files

[ii...@apache.org]

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

iilyak 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 69d15cd  Refactor fabric:cleanup_index_files
     new 5a6c944  Merge pull request #2101 from cloudant/refactor-clean_index_files
69d15cd is described below

commit 69d15cd10652de1895b7750dd86b37ba04267f1d
Author: ILYA Khlopotov <ii...@apache.org>
AuthorDate: Wed Aug 7 16:45:15 2019 +0000

    Refactor fabric:cleanup_index_files
    
    Previous implementation assembled a regexp by concatenating active signatures.
    The approach caused regexp to exceed system limit in the case of huge number of them.
---
 src/couch/test/exunit/fabric_test.exs | 101 ++++++++++++++++++++++++++++++++++
 src/fabric/src/fabric.erl             |  25 +++++----
 test/elixir/lib/setup/common.ex       |   5 ++
 3 files changed, 121 insertions(+), 10 deletions(-)

diff --git a/src/couch/test/exunit/fabric_test.exs b/src/couch/test/exunit/fabric_test.exs
new file mode 100644
index 0000000..bdb84e9
--- /dev/null
+++ b/src/couch/test/exunit/fabric_test.exs
@@ -0,0 +1,101 @@
+defmodule Couch.Test.Fabric do
+  use Couch.Test.ExUnit.Case
+  alias Couch.Test.Utils
+
+  alias Couch.Test.Setup
+
+  alias Couch.Test.Setup.Step
+
+  import Couch.DBTest
+
+  import Utils
+
+  @admin {:user_ctx, user_ctx(roles: ["_admin"])}
+
+  def with_db(context, setup) do
+    setup =
+      setup
+      |> Setup.Common.with_db()
+      |> Setup.run()
+
+    context =
+      Map.merge(context, %{
+        db_name: setup |> Setup.get(:db) |> Step.Create.DB.name()
+      })
+
+    {context, setup}
+  end
+
+  describe "Fabric miscellaneous API" do
+    @describetag setup: &__MODULE__.with_db/2
+    test "Get inactive_index_files", ctx do
+      {:ok, _rev} = update_doc(ctx.db_name, %{"_id" => "doc1"})
+
+      design_doc = %{
+        "_id" => "_design/test",
+        "language" => "javascript",
+        "views" => %{
+          "view" => %{
+            "map" => "function(doc){emit(doc._id, doc._rev)}"
+          }
+        }
+      }
+
+      {:ok, rev1} = update_doc(ctx.db_name, design_doc)
+      wait_sig_update(ctx.db_name, "test", "")
+      prev_active = get_active_sig(ctx.db_name, "test")
+
+      updated_design_doc =
+        put_in(design_doc, ["views", "view", "map"], "function(doc){emit(doc._id, null)}")
+
+      {:ok, rev2} =
+        update_doc(
+          ctx.db_name,
+          Map.put(updated_design_doc, "_rev", rev1)
+        )
+
+      assert rev1 != rev2
+      wait_sig_update(ctx.db_name, "test", prev_active)
+
+      {:ok, info} = :fabric.get_view_group_info(ctx.db_name, "_design/test")
+      active = info[:signature]
+
+      files = Enum.map(:fabric.inactive_index_files(ctx.db_name), &List.to_string/1)
+
+      assert [] != files, "We should have some inactive"
+
+      assert not Enum.any?(files, fn
+               file_path -> String.contains?(file_path, active)
+             end),
+             "We are not suppose to return active views"
+
+      assert Enum.all?(files, fn
+               file_path -> String.contains?(file_path, prev_active)
+             end),
+             "We expect all files to contain previous active signature"
+    end
+  end
+
+  defp update_doc(db_name, body) do
+    json_body = :jiffy.decode(:jiffy.encode(body))
+
+    case :fabric.update_doc(db_name, json_body, [@admin]) do
+      {:ok, rev} ->
+        {:ok, :couch_doc.rev_to_str(rev)}
+
+      error ->
+        error
+    end
+  end
+
+  defp get_active_sig(db_name, ddoc_id) do
+    {:ok, info} = :fabric.get_view_group_info(db_name, "_design/#{ddoc_id}")
+    info[:signature]
+  end
+
+  defp wait_sig_update(db_name, ddoc_id, prev_active) do
+    retry_until(fn ->
+      get_active_sig(db_name, ddoc_id) != prev_active
+    end)
+  end
+end
diff --git a/src/fabric/src/fabric.erl b/src/fabric/src/fabric.erl
index 6d04184..d98ffc9 100644
--- a/src/fabric/src/fabric.erl
+++ b/src/fabric/src/fabric.erl
@@ -36,7 +36,8 @@
 
 % miscellany
 -export([design_docs/1, reset_validation_funs/1, cleanup_index_files/0,
-    cleanup_index_files/1, cleanup_index_files_all_nodes/1, dbname/1]).
+    cleanup_index_files/1, cleanup_index_files_all_nodes/1, dbname/1,
+    inactive_index_files/1]).
 
 -include_lib("fabric/include/fabric.hrl").
 
@@ -503,26 +504,30 @@ cleanup_index_files() ->
 %% @doc clean up index files for a specific db
 -spec cleanup_index_files(dbname()) -> ok.
 cleanup_index_files(DbName) ->
+    lists:foreach(fun(File) ->
+        file:delete(File)
+    end, inactive_index_files(DbName)).
+
+%% @doc inactive index files for a specific db
+-spec inactive_index_files(dbname()) -> ok.
+inactive_index_files(DbName) ->
     {ok, DesignDocs} = fabric:design_docs(DbName),
 
-    ActiveSigs = lists:map(fun(#doc{id = GroupId}) ->
+    ActiveSigs = maps:from_list(lists:map(fun(#doc{id = GroupId}) ->
         {ok, Info} = fabric:get_view_group_info(DbName, GroupId),
-        binary_to_list(couch_util:get_value(signature, Info))
-    end, [couch_doc:from_json_obj(DD) || DD <- DesignDocs]),
+        {binary_to_list(couch_util:get_value(signature, Info)), nil}
+    end, [couch_doc:from_json_obj(DD) || DD <- DesignDocs])),
 
     FileList = lists:flatmap(fun(#shard{name = ShardName}) ->
         IndexDir = couch_index_util:index_dir(mrview, ShardName),
         filelib:wildcard([IndexDir, "/*"])
     end, mem3:local_shards(dbname(DbName))),
 
-    DeleteFiles = if ActiveSigs =:= [] -> FileList; true ->
-        {ok, RegExp} = re:compile([$(, string:join(ActiveSigs, "|"), $)]),
+    if ActiveSigs =:= [] -> FileList; true ->
         lists:filter(fun(FilePath) ->
-            re:run(FilePath, RegExp, [{capture, none}]) == nomatch
+            not maps:is_key(filename:basename(FilePath, ".view"), ActiveSigs)
         end, FileList)
-    end,
-    [file:delete(File) || File <- DeleteFiles],
-    ok.
+    end.
 
 %% @doc clean up index files for a specific db on all nodes
 -spec cleanup_index_files_all_nodes(dbname()) -> [reference()].
diff --git a/test/elixir/lib/setup/common.ex b/test/elixir/lib/setup/common.ex
index 3b59e94..e81f109 100644
--- a/test/elixir/lib/setup/common.ex
+++ b/test/elixir/lib/setup/common.ex
@@ -19,4 +19,9 @@ defmodule Couch.Test.Setup.Common do
       |> Step.Create.DB.new(:db)
   end
 
+  def with_db(setup) do
+    setup
+      |> Step.Start.new(:start, extra_apps: [:fabric])
+      |> Step.Create.DB.new(:db)
+  end
 end
\ No newline at end of file