Просмотр исходного кода

Do not return errors on metadata update when strict_mode is not enabled.

Roberto Ostinelli 3 лет назад
Родитель
Сommit
e9c770dc76
8 измененных файлов с 80 добавлено и 158 удалено
  1. 20 47
      OPTIONS.md
  2. 10 13
      src/syn.erl
  3. 0 3
      src/syn_event_handler.erl
  4. 1 7
      src/syn_pg.erl
  5. 1 7
      src/syn_registry.erl
  6. 27 44
      test/syn_pg_SUITE.erl
  7. 21 29
      test/syn_registry_SUITE.erl
  8. 0 8
      test/syn_test_suite_helper.erl

+ 20 - 47
OPTIONS.md

@@ -44,47 +44,37 @@ config :syn,
 ```
 
 ## strict_mode
-By default, Syn doesn't enforce which process performs the Registry and Process Groups operations:
+By default, Syn doesn't enforce which processes perform the Registry and Process Groups operations:
 for instance, a process can be registered by a call running in another process.
-While this is fine in standard operations, those that would end up updating a process' metadata
-return a `{error, non_strict_update}` tuple when `strict_mode` is not enabled (the default).
-
-This is because enabling any process to update another process' metadata could lead to potential race conditions.
+While this is fine in standard operations, those that end up updating a process' metadata could potentially
+lead to race conditions if caution is not taken.
 
 Operations are serialized by the [authority node](internals.html#node-authority) responsible for a process, however
-simultaneous requests coming from different processes to update a specific process' metadata would result
-in having the metadata of the first incoming request to be overwritten by the second request.
-While this doesn't compromise Syn's strict eventual consistency (i.e. the data across the cluster will eventually converge),
-an application might overwrite metadata and have unexpected logical consequences.
+simultaneous requests coming from different processes to update a specific process' metadata can result
+in unexpected consequences, even if this doesn't compromise Syn's strict eventual consistency
+(i.e. the data across the cluster will still eventually converge).
 
-Imagine for instance that a process A has a metadata of attributes, such as `[{color, "blue"}]`. A different process
+Imagine for instance that a process A has a metadata of attributes such as `[{color, "blue"}]`. A different process
 might request to update process A's metadata to include size, by setting its metadata to `[{color, "blue"}, {size, 100}]`.
-At almost the same time another process issues the request to update process A's metadata to include weight, by setting
-its metadata to `[{color, "blue"}, {weight, 1.0}]`. The first request to set size will then be overwritten shortly after
-by the second incoming request to set the weight, thus resulting in the loss of the size information,
-and the process A' metadata will be propagated to `[{color, "blue"}, {weight, 1.0}]` in all the cluster.
+At almost the same time another process might also request to update process A's metadata to include weight, by setting
+its metadata to `[{color, "blue"}, {weight, 1.0}]`.
+
+These requests will be delivered to the authority node which will treat them sequentially.
+Therefore, the first incoming request (for eg. the one to set size) will be overwritten shortly after
+by the second incoming request (to set the weight), thus resulting in the loss of the `{size, 100}` tuple
+in the process' metadata.  The end result will be that process A' metadata will be propagated to the whole cluster as
+`[{color, "blue"}, {weight, 1.0}]`.
+
+This can be circumvented by proper application design, for instance by ensuring that a single process
+is always responsible for updating a specific process' metadata.
 
-For this reason, if an application needs to update process' metadata, `strict_mode` needs to be turned on.
-When enabled, only a process can perform Registry and Process Groups operations for itself.
+When enabled, Syn's `strict_mode` is a helper to enforce that a process can only update its own metadata.
 This basically means that the `Pid` parameter of most methods must be `self()`.
 
-This is a global setting that cannot be specified on a per-scope basis. The same setting SHOULD be set
+`strict_mode` is a global setting that cannot be specified on a per-scope basis. The same setting SHOULD be set
 on every Erlang cluster node running Syn.
 
 #### Elixir
-With `strict_mode` turned off (the default):
-
-```elixir
-iex> pid = spawn(fn -> receive do _ -> :ok end end)
-#PID<0.108.0>
-iex> :syn.register(:users, "hedy", pid).
-:ok
-iex> :syn.register(:users, "hedy", pid).
-ok
-iex> :syn.register(:users, "hedy", pid, :new_metadata).
-{:error, :non_strict_update}
-```
-
 With `strict_mode` turned on:
 
 ```elixir
@@ -94,24 +84,9 @@ iex> :syn.register(:users, "hedy", pid).
 {:error, :not_self}
 iex> :syn.register(:users, "hedy", self()).
 :ok
-iex> :syn.register(:users, "hedy", self(), :new_metadata).
-:ok
 ```
 
 #### Erlang
-With `strict_mode` turned off (the default):
-
-```erlang
-1> Pid = spawn(fun() -> receive _ -> ok end end).
-<0.83.0>
-2> syn:register(users, "hedy", Pid).
-ok
-3> syn:register(users, "hedy", Pid).
-ok
-4> syn:register(users, "hedy", Pid, new_metadata).
-{error, non_strict_update}
-```
-
 With `strict_mode` turned on:
 
 ```erlang
@@ -121,8 +96,6 @@ With `strict_mode` turned on:
 {error, not_self}
 3> syn:register(users, "hedy", self()).
 ok
-4> syn:register(users, "hedy", self(), new_metadata).
-ok
 ```
 
 `strict_mode` can be turned on by setting the `strict_mode` configuration variable.

+ 10 - 13
src/syn.erl

@@ -286,9 +286,10 @@ register(Scope, Name, Pid) ->
 
 %% @doc Registers a process with metadata in the specified Scope.
 %%
-%% You may re-register a process multiple times, and you may also register the same process with different names.
-%%
-%% To update a process' metadata, <a href="options.html#strict_mode">`strict_mode'</a> must be enabled.
+%% You may register the same process with different names.
+%% You may also re-register a process multiple times, for example if you need to update its metadata, however it is
+%% RECOMMENDED to be aware of the implications of updating metadata, see the <a href="options.html#strict_mode">`strict_mode'</a>
+%% option for more information.
 %%
 %% When a process gets registered, Syn will automatically monitor it.
 %%
@@ -296,9 +297,7 @@ register(Scope, Name, Pid) ->
 %% <ul>
 %% <li>`not_alive': The `pid()' being registered is not alive.</li>
 %% <li>`taken': name is already registered with another `pid()'.</li>
-%% <li>`non_strict_update': process is already registered with different metadata,
-%% but <a href="options.html#strict_mode">`strict_mode'</a> is not enabled.</li>
-%% <li>`not_self': the method is called from a process other than `self()',
+%% <li>`not_self': the method is being called from a process other than `self()',
 %% but <a href="options.html#strict_mode">`strict_mode'</a> is enabled.</li>
 %% </ul>
 %%
@@ -469,19 +468,17 @@ join(Scope, GroupName, Pid) ->
 
 %% @doc Adds a `pid()' with metadata to GroupName in the specified Scope.
 %%
-%% A process can join multiple groups. A process may join the same group multiple times, though it will still be
-%% listed only once in it.
-%%
-%% To update a process' metadata, <a href="options.html#strict_mode">`strict_mode'</a> must be enabled.
+%% A process can join multiple groups.
+%% A process may also join the same group multiple times, for example if you need to update its metadata, however it is
+%% RECOMMENDED to be aware of the implications of updating metadata, see the <a href="options.html#strict_mode">`strict_mode'</a>
+%% option for more information.
 %%
 %% When a process joins a group, Syn will automatically monitor it.
 %%
 %% Possible error reasons:
 %% <ul>
 %% <li>`not_alive': The `pid()' being added is not alive.</li>
-%% <li>`non_strict_update': process is already in group with different metadata,
-%% but <a href="options.html#strict_mode">`strict_mode'</a> is not enabled.</li>
-%% <li>`not_self': the method is called from a process other than `self()',
+%% <li>`not_self': the method is being called from a process other than `self()',
 %% but <a href="options.html#strict_mode">`strict_mode'</a> is enabled.</li>
 %% </ul>
 %%

+ 0 - 3
src/syn_event_handler.erl

@@ -37,9 +37,6 @@
 %% All of the callbacks, except for `resolve_registry_conflict/4', are called on all the nodes of the cluster.
 %% This allows you to receive events for the processes running on nodes that get shut down, or in case of net splits.
 %%
-%% For the `on_registry_process_updated/5' and `on_group_process_updated/5' callbacks to be called,
-%% <a href="options.html#strict_mode">`strict_mode'</a> must be enabled.
-%%
 %% The argument `Reason' in the callbacks can be:
 %% <ul>
 %% <li> `normal' for expected operations.</li>

+ 1 - 7
src/syn_pg.erl

@@ -291,13 +291,7 @@ handle_call({'3.0', join_on_node, RequesterNode, GroupName, Pid, Meta}, _From, #
 
                 {{_, _}, _, _, MRef, _} ->
                     %% re-joined with different meta
-                    case syn_backbone:is_strict_mode() of
-                        true ->
-                            do_join_on_node(GroupName, Pid, Meta, MRef, normal, RequesterNode, on_group_process_updated, State);
-
-                        false ->
-                            {reply, {{error, non_strict_update}, undefined}, State}
-                    end
+                    do_join_on_node(GroupName, Pid, Meta, MRef, normal, RequesterNode, on_group_process_updated, State)
             end;
 
         false ->

+ 1 - 7
src/syn_registry.erl

@@ -205,13 +205,7 @@ handle_call({'3.0', register_on_node, RequesterNode, Name, Pid, Meta}, _From, #s
 
                 {Name, Pid, _, _, MRef, _} ->
                     %% same pid, different meta
-                    case syn_backbone:is_strict_mode() of
-                        true ->
-                            do_register_on_node(Name, Pid, Meta, MRef, normal, RequesterNode, on_registry_process_updated, State);
-
-                        false ->
-                            {reply, {{error, non_strict_update}, undefined}, State}
-                    end;
+                    do_register_on_node(Name, Pid, Meta, MRef, normal, RequesterNode, on_registry_process_updated, State);
 
                 _ ->
                     {reply, {{error, taken}, undefined}, State}

+ 27 - 44
test/syn_pg_SUITE.erl

@@ -197,27 +197,20 @@ one_node_strict_mode(_Config) ->
     ok = syn:start(),
     syn:add_node_to_scopes([scope]),
 
+    %% strict mode enabled
+    application:set_env(syn, strict_mode, true),
+
     %% start process
     Pid = syn_test_suite_helper:start_process(),
+    {error, not_self} = syn:join(scope, "strict", Pid, metadata),
 
-    %% strict mode disabled
-    ok = syn:join(scope, "strict-false", Pid, metadata),
-    ok = syn:join(scope, "strict-false", Pid, metadata),
-    {error, non_strict_update} = syn:join(scope, "strict-false", Pid, new_metadata),
-    ok = syn:join(scope, "strict-false", Pid, metadata),
-    [{Pid, metadata}] = syn:members(scope, "strict-false"),
-    ok = syn:leave(scope, "strict-false", Pid),
-
-    %% strict mode enabled
-    application:set_env(syn, strict_mode, true),
-    {error, not_self} = syn:join(scope, "strict-true", Pid, metadata),
     Self = self(),
-    ok = syn:join(scope, "strict-true", Self, metadata),
-    ok = syn:join(scope, "strict-true", Self, new_metadata),
-    [{Self, new_metadata}] = syn:members(scope, "strict-true"),
-    ok = syn:join(scope, "strict-true", Self),
-    [{Self, undefined}] = syn:members(scope, "strict-true"),
-    ok = syn:leave(scope, "strict-true", Self).
+    ok = syn:join(scope, "strict", Self, metadata),
+    ok = syn:join(scope, "strict", Self, new_metadata),
+    [{Self, new_metadata}] = syn:members(scope, "strict"),
+    ok = syn:join(scope, "strict", Self),
+    [{Self, undefined}] = syn:members(scope, "strict"),
+    ok = syn:leave(scope, "strict", Self).
 
 three_nodes_discover(Config) ->
     %% get slaves
@@ -512,13 +505,9 @@ three_nodes_join_leave_and_monitor(Config) ->
     1 = rpc:call(SlaveNode2, syn, group_count, [scope_bc, SlaveNode1]),
     0 = rpc:call(SlaveNode2, syn, group_count, [scope_bc, SlaveNode2]),
 
-    %% enable strict to allow for updates
-    application:set_env(syn, strict_mode, true),
-    rpc:call(SlaveNode1, application, set_env, [syn, strict_mode, true]),
-
-    %% re-join to edit meta (strict is enabled, so send message)
-    PidWithMeta ! {pg_update_meta, scope_ab, {group, "one"}, <<"with updated meta">>},
-    PidRemoteOn1 ! {pg_update_meta, scope_bc, {group, "two"}, added_meta},
+    %% re-join to edit meta
+    ok = syn:join(scope_ab, {group, "one"}, PidWithMeta, <<"with updated meta">>),
+    ok = rpc:call(SlaveNode2, syn, join, [scope_bc, {group, "two"}, PidRemoteOn1, added_meta]), %% updated on slave 2
 
     %% retrieve
     syn_test_suite_helper:assert_wait(
@@ -606,10 +595,6 @@ three_nodes_join_leave_and_monitor(Config) ->
     1 = rpc:call(SlaveNode2, syn, group_count, [scope_bc, SlaveNode1]),
     0 = rpc:call(SlaveNode2, syn, group_count, [scope_bc, SlaveNode2]),
 
-    %% disable strict
-    application:set_env(syn, strict_mode, false),
-    rpc:call(SlaveNode1, application, set_env, [syn, strict_mode, false]),
-
     ok = syn:join(scope_ab, {group, "two"}, PidRemoteOn1),
     syn_test_suite_helper:assert_wait(
         lists:sort([{Pid, undefined}, {PidWithMeta, "with-meta-2"}, {PidRemoteOn1, undefined}]),
@@ -1165,11 +1150,8 @@ three_nodes_custom_event_handler_joined_left(Config) ->
         {on_process_joined, SlaveNode2, scope_all, "my-group", Pid2, <<"meta-for-2">>, normal}
     ]),
 
-    %% enable strict to allow for updates
-    application:set_env(syn, strict_mode, true),
-
-    %% ---> on meta update (strict is enabled, so send message)
-    Pid ! {pg_update_meta, scope_all, "my-group", {recipient, self(), <<"new-meta-0">>}},
+    %% ---> on meta update
+    ok = syn:join(scope_all, "my-group", Pid, {recipient, self(), <<"new-meta-0">>}),
 
     %% check callbacks called
     syn_test_suite_helper:assert_received_messages([
@@ -1178,17 +1160,24 @@ three_nodes_custom_event_handler_joined_left(Config) ->
         {on_group_process_updated, SlaveNode2, scope_all, "my-group", Pid, <<"new-meta-0">>, normal}
     ]),
 
-    %% disable strict
-    application:set_env(syn, strict_mode, false),
+    %% update meta from another node
+    ok = rpc:call(SlaveNode1, syn, join, [scope_all, "my-group", Pid, {recipient, self(), <<"new-meta">>}]),
+
+    %% check callbacks called
+    syn_test_suite_helper:assert_received_messages([
+        {on_group_process_updated, LocalNode, scope_all, "my-group", Pid, <<"new-meta">>, normal},
+        {on_group_process_updated, SlaveNode1, scope_all, "my-group", Pid, <<"new-meta">>, normal},
+        {on_group_process_updated, SlaveNode2, scope_all, "my-group", Pid, <<"new-meta">>, normal}
+    ]),
 
     %% ---> on left
     ok = syn:leave(scope_all, "my-group", Pid),
 
     %% check callbacks called
     syn_test_suite_helper:assert_received_messages([
-        {on_process_left, LocalNode, scope_all, "my-group", Pid, <<"new-meta-0">>, normal},
-        {on_process_left, SlaveNode1, scope_all, "my-group", Pid, <<"new-meta-0">>, normal},
-        {on_process_left, SlaveNode2, scope_all, "my-group", Pid, <<"new-meta-0">>, normal}
+        {on_process_left, LocalNode, scope_all, "my-group", Pid, <<"new-meta">>, normal},
+        {on_process_left, SlaveNode1, scope_all, "my-group", Pid, <<"new-meta">>, normal},
+        {on_process_left, SlaveNode2, scope_all, "my-group", Pid, <<"new-meta">>, normal}
     ]),
 
     %% leave from another node
@@ -1669,12 +1658,6 @@ four_nodes_concurrency(Config) ->
     ok = rpc:call(SlaveNode2, syn, start, []),
     ok = rpc:call(SlaveNode3, syn, start, []),
 
-    %% enable strict to allow for updates
-    application:set_env(syn, strict_mode, true),
-    rpc:call(SlaveNode1, application, set_env, [syn, strict_mode, true]),
-    rpc:call(SlaveNode2, application, set_env, [syn, strict_mode, true]),
-    rpc:call(SlaveNode3, application, set_env, [syn, strict_mode, true]),
-
     %% add scopes
     ok = syn:add_node_to_scopes([scope_all]),
     ok = rpc:call(SlaveNode1, syn, add_node_to_scopes, [[scope_all]]),

+ 21 - 29
test/syn_registry_SUITE.erl

@@ -222,19 +222,13 @@ one_node_strict_mode(_Config) ->
     ok = syn:start(),
     syn:add_node_to_scopes([scope]),
 
-    %% start process
-    Pid = syn_test_suite_helper:start_process(),
-
-    %% strict mode disabled
-    ok = syn:register(scope, "strict-false", Pid, metadata),
-    ok = syn:register(scope, "strict-false", Pid, metadata),
-    {error, non_strict_update} = syn:register(scope, "strict-false", Pid, new_metadata),
-    {Pid, metadata} = syn:lookup(scope, "strict-false"),
-    ok = syn:register(scope, "strict-false", Pid, metadata),
-
     %% strict mode enabled
     application:set_env(syn, strict_mode, true),
+
+    %% start process
+    Pid = syn_test_suite_helper:start_process(),
     {error, not_self} = syn:register(scope, "strict-true", Pid, metadata),
+
     Self = self(),
     ok = syn:register(scope, "strict-true", Self, metadata),
     ok = syn:register(scope, "strict-true", Self, new_metadata),
@@ -454,11 +448,8 @@ three_nodes_register_unregister_and_monitor(Config) ->
     1 = rpc:call(SlaveNode2, syn, registry_count, [scope_bc, SlaveNode1]),
     0 = rpc:call(SlaveNode2, syn, registry_count, [scope_bc, SlaveNode2]),
 
-    %% enable strict to allow for updates
-    application:set_env(syn, strict_mode, true),
-
-    %% re-register to edit meta (strict is enabled, so send message)
-    PidWithMeta ! {registry_update_meta, scope_ab, "scope_a_alias", <<"with_meta_updated">>},
+    %% re-register to edit meta
+    ok = syn:register(scope_ab, "scope_a_alias", PidWithMeta, <<"with_meta_updated">>),
 
     syn_test_suite_helper:assert_wait(
         {PidWithMeta, <<"with_meta_updated">>},
@@ -470,9 +461,6 @@ three_nodes_register_unregister_and_monitor(Config) ->
     ),
     {badrpc, {'EXIT', {{invalid_scope, scope_ab}, _}}} = (catch rpc:call(SlaveNode2, syn, lookup, [scope_ab, "scope_a_alias"])),
 
-    %% disable strict
-    application:set_env(syn, strict_mode, false),
-
     %% register remote
     ok = syn:register(scope_ab, "ab_on_1", PidRemoteWithMetaOn1, <<"ab-on-1">>),
     syn_test_suite_helper:assert_wait(
@@ -1000,11 +988,8 @@ three_nodes_custom_event_handler_reg_unreg(Config) ->
         {on_process_registered, SlaveNode2, scope_all, "proc-handler-2", Pid2, <<"meta-for-2">>, normal}
     ]),
 
-    %% enable strict to allow for updates
-    application:set_env(syn, strict_mode, true),
-
-    %% ---> on meta update (strict is enabled, so send message)
-    Pid ! {registry_update_meta, scope_all, "proc-handler", {recipient, self(), <<"new-meta">>}},
+    %% ---> on meta update
+    ok = syn:register(scope_all, "proc-handler", Pid, {recipient, self(), <<"new-meta">>}),
 
     %% check callbacks called
     syn_test_suite_helper:assert_received_messages([
@@ -1013,6 +998,16 @@ three_nodes_custom_event_handler_reg_unreg(Config) ->
         {on_registry_process_updated, SlaveNode2, scope_all, "proc-handler", Pid, <<"new-meta">>, normal}
     ]),
 
+    %% meta update from another node
+    ok = rpc:call(SlaveNode1, syn, register, [scope_all, "proc-handler-2", Pid2, {recipient, self(), <<"meta-for-2-update">>}]),
+
+    %% check callbacks called
+    syn_test_suite_helper:assert_received_messages([
+        {on_registry_process_updated, LocalNode, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal},
+        {on_registry_process_updated, SlaveNode1, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal},
+        {on_registry_process_updated, SlaveNode2, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal}
+    ]),
+
     %% ---> on unregister
     ok = syn:unregister(scope_all, "proc-handler"),
 
@@ -1028,9 +1023,9 @@ three_nodes_custom_event_handler_reg_unreg(Config) ->
 
     %% check callbacks called
     syn_test_suite_helper:assert_received_messages([
-        {on_process_unregistered, LocalNode, scope_all, "proc-handler-2", Pid2, <<"meta-for-2">>, normal},
-        {on_process_unregistered, SlaveNode1, scope_all, "proc-handler-2", Pid2, <<"meta-for-2">>, normal},
-        {on_process_unregistered, SlaveNode2, scope_all, "proc-handler-2", Pid2, <<"meta-for-2">>, normal}
+        {on_process_unregistered, LocalNode, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal},
+        {on_process_unregistered, SlaveNode1, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal},
+        {on_process_unregistered, SlaveNode2, scope_all, "proc-handler-2", Pid2, <<"meta-for-2-update">>, normal}
     ]),
 
     %% clean & check
@@ -1038,9 +1033,6 @@ three_nodes_custom_event_handler_reg_unreg(Config) ->
     %% no messages
     syn_test_suite_helper:assert_empty_queue(),
 
-    %% disable strict
-    application:set_env(syn, strict_mode, false),
-
     %% ---> after a netsplit
     PidRemoteOn1 = syn_test_suite_helper:start_process(SlaveNode1),
     ok = syn:register(scope_all, remote_on_1, PidRemoteOn1, {recipient, self(), <<"netsplit">>}),

+ 0 - 8
test/syn_test_suite_helper.erl

@@ -316,14 +316,6 @@ send_error_logger_to_disk() ->
 %% ===================================================================
 process_main() ->
     receive
-        {registry_update_meta, Scope, Name, NewMeta} ->
-            ok = syn:register(Scope, Name, self(), NewMeta),
-            process_main();
-
-        {pg_update_meta, Scope, GroupName, NewMeta} ->
-            ok = syn:join(Scope, GroupName, self(), NewMeta),
-            process_main();
-
         _ ->
             process_main()
     end.