The race condition between providers is fixed 4/head
authorILYA Khlopotov <iilyak@ca.ibm.com>
Tue, 14 Jul 2015 15:27:37 +0000 (08:27 -0700)
committerILYA Khlopotov <iilyak@ca.ibm.com>
Fri, 17 Jul 2015 17:11:25 +0000 (10:11 -0700)
In previous design there is a race condition between the time we read the
current definitions in the generated module and the time we compile new
version of it. This race led to overwriting of data of already
configured providers.

This commit fixes the issue by introducing couch_epi_module_keeper
process. This process is named by generated module name and it is
essentially a requests serializer.

src/couch_epi_data.erl
src/couch_epi_data_gen.erl
src/couch_epi_functions.erl
src/couch_epi_functions_gen.erl
src/couch_epi_keeper_sup.erl [new file with mode: 0644]
src/couch_epi_module_keeper.erl [new file with mode: 0644]
src/couch_epi_sup.erl

index d4211333cd36fb67f572f38e305927b24561d576..680e5f5ae311991c094c01c5c17a667f6aa95230 100644 (file)
@@ -53,6 +53,7 @@ childspec(Id, App, EpiKey, Module) ->
     }.
 
 start_link(SubscriberApp, {epi_key, Key}, Module, Options) ->
+    maybe_start_keeper(Key),
     gen_server:start_link(?MODULE, [SubscriberApp, Module, Key, Options], []).
 
 reload(Server) ->
@@ -149,3 +150,7 @@ current(Handle, Subscriber) ->
     catch error:undef ->
         []
     end.
+
+maybe_start_keeper(Key) ->
+    Handle = couch_epi_data_gen:get_handle(Key),
+    couch_epi_module_keeper:maybe_start_keeper(couch_epi_data_gen, Handle).
index 73cf9016c1f59229da663e6b42bf2bd7c68570b0..75601cf255f568851bca937545c6f464383e29b9 100644 (file)
 -export([by_source/1, by_source/2]).
 -export([keys/1, subscribers/1]).
 
+-export([save/3]).
+
 set(Handle, Source, Data) ->
     case is_updated(Handle, Source, Data) of
         false ->
             ok;
         true ->
-            save(Handle, Source, Data)
+            couch_epi_module_keeper:save(Handle, Source, Data)
     end.
 
 get(Handle) ->
index 86687a7c47d5df6f47cb7af166c3e31983bc79b8..09ece2e529bb8306e9e6959e08e79225e15f24ed 100644 (file)
@@ -53,6 +53,7 @@ childspec(Id, App, Key, Module) ->
     }.
 
 start_link(ProviderApp, {epi_key, ServiceId}, {modules, Modules}, Options) ->
+    maybe_start_keeper(ServiceId),
     gen_server:start_link(
         ?MODULE, [ProviderApp, ServiceId, Modules, Options], []).
 
@@ -148,3 +149,7 @@ safe_remove(#state{} = State) ->
     catch Class:Reason ->
         {{Class, Reason}, State}
     end.
+
+maybe_start_keeper(ServiceId) ->
+    Handle = couch_epi_functions_gen:get_handle(ServiceId),
+    couch_epi_module_keeper:maybe_start_keeper(couch_epi_functions_gen, Handle).
index b62fbb5a8c065c730b5fca7d498171dd88bd6f04..a08573c77b740ae975e0859ed16cddc37c91f23a 100644 (file)
@@ -14,6 +14,8 @@
 
 -export([add/3, remove/3, get_handle/1, hash/1, apply/4, apply/5]).
 
+-export([save/3]).
+
 -ifdef(TEST).
 
 -export([foo/2, bar/0]).
@@ -32,7 +34,7 @@ add(Handle, Source, Modules) ->
         false ->
             ok;
         true ->
-            save(Handle, Source, Modules)
+            couch_epi_module_keeper:save(Handle, Source, Modules)
     end.
 
 remove(Handle, Source, Modules) ->
diff --git a/src/couch_epi_keeper_sup.erl b/src/couch_epi_keeper_sup.erl
new file mode 100644 (file)
index 0000000..c342d68
--- /dev/null
@@ -0,0 +1,58 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_epi_keeper_sup).
+
+-behaviour(supervisor).
+
+%% ------------------------------------------------------------------
+%% API Function Exports
+%% ------------------------------------------------------------------
+
+-export([start_link/0]).
+
+-export([start_child/2, terminate_child/1]).
+
+%% ------------------------------------------------------------------
+%% supervisor Function Exports
+%% ------------------------------------------------------------------
+
+-export([init/1]).
+
+%% Helper macro for declaring children of supervisor
+-define(CHILD(I, Type), {I, {I, start_link, []}, permanent, 5000, Type, [I]}).
+
+%% ===================================================================
+%% API functions
+%% ===================================================================
+
+start_link() ->
+    supervisor:start_link({local, ?MODULE}, ?MODULE, []).
+
+start_child(Codegen, Module) ->
+    supervisor:start_child(?MODULE, [Codegen, Module]).
+
+terminate_child(undefined) -> ok;
+terminate_child(Child) when is_atom(Child) ->
+    terminate_child(whereis(Child));
+terminate_child(ChildPid) ->
+    supervisor:terminate_child(?MODULE, ChildPid).
+
+%% ===================================================================
+%% Supervisor callbacks
+%% ===================================================================
+
+init([]) ->
+    Children = [
+        ?CHILD(couch_epi_module_keeper, worker)
+    ],
+    {ok, { {simple_one_for_one, 5, 10}, Children} }.
diff --git a/src/couch_epi_module_keeper.erl b/src/couch_epi_module_keeper.erl
new file mode 100644 (file)
index 0000000..f538cf5
--- /dev/null
@@ -0,0 +1,78 @@
+% Licensed under the Apache License, Version 2.0 (the "License"); you may not
+% use this file except in compliance with the License. You may obtain a copy of
+% the License at
+%
+% http://www.apache.org/licenses/LICENSE-2.0
+%
+% Unless required by applicable law or agreed to in writing, software
+% distributed under the License is distributed on an "AS IS" BASIS, WITHOUT
+% WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the
+% License for the specific language governing permissions and limitations under
+% the License.
+
+-module(couch_epi_module_keeper).
+
+-behaviour(gen_server).
+
+%% ------------------------------------------------------------------
+%% API Function Exports
+%% ------------------------------------------------------------------
+
+-export([maybe_start_keeper/2]).
+-export([start_link/2, save/3]).
+-export([stop/1]).
+
+%% ------------------------------------------------------------------
+%% gen_server Function Exports
+%% ------------------------------------------------------------------
+
+-export([init/1, handle_call/3, handle_cast/2, handle_info/2,
+         terminate/2, code_change/3]).
+
+-record(state, {codegen, module}).
+
+%% ------------------------------------------------------------------
+%% API Function Definitions
+%% ------------------------------------------------------------------
+
+maybe_start_keeper(Codegen, Module) ->
+    catch couch_epi_keeper_sup:start_child(Codegen, Module).
+
+start_link(Codegen, Module) ->
+    gen_server:start_link({local, Module}, ?MODULE, [Codegen, Module], []).
+
+stop(Server) ->
+    catch gen_server:call(Server, stop).
+
+save(Server, Source, Config) ->
+    gen_server:call(Server, {save, Source, Config}).
+
+%% ------------------------------------------------------------------
+%% gen_server Function Definitions
+%% ------------------------------------------------------------------
+
+init([Codegen, Module]) ->
+    {ok, #state{codegen = Codegen, module = Module}}.
+
+handle_call({save, Source, Config}, _From, State) ->
+    #state{codegen = Codegen, module = Module} = State,
+    Reply = Codegen:save(Module, Source, Config),
+    {reply, Reply, State};
+handle_call(_Request, _From, State) ->
+    {reply, ok, State}.
+
+handle_cast(_Msg, State) ->
+    {noreply, State}.
+
+handle_info(_Info, State) ->
+    {noreply, State}.
+
+terminate(_Reason, _State) ->
+    ok.
+
+code_change(_OldVsn, State, _Extra) ->
+    {ok, State}.
+
+%% ------------------------------------------------------------------
+%% Internal Function Definitions
+%% ------------------------------------------------------------------
index 5e44d1b012e4ad03bf7d7d73b7de62c780a292d3..3c35d2dd0a832c1706f126fe24a93074466fd1b3 100644 (file)
@@ -22,6 +22,8 @@
 
 %% Helper macro for declaring children of supervisor
 -define(CHILD(I, Type), {I, {I, start_link, []}, permanent, 5000, Type, [I]}).
+-define(SUP(I, A),
+        {I, {I, start_link, A}, permanent, infinity, supervisor, [I]}).
 
 %% ===================================================================
 %% API functions
@@ -35,4 +37,8 @@ start_link() ->
 %% ===================================================================
 
 init([]) ->
-    {ok, { {one_for_one, 5, 10}, [?CHILD(couch_epi_server, worker)]} }.
+    Children = [
+        ?CHILD(couch_epi_server, worker),
+        ?SUP(couch_epi_keeper_sup, [])
+    ],
+    {ok, { {one_for_one, 5, 10}, Children} }.