Merge branch 'anders/diameter/counters/OTP-16457' into maint-22

* anders/diameter/counters/OTP-16457:
  Tweak traffic counters for remote request handler processes
  Fix/tweak dialyze target
  Fix diameter:service_info/2 spec
  Add/fix comments
  Fix dialyzer diameter_tcp:l/2 warning
  Relay remote answers more efficiently
  Send requests on remote transport more efficiently
  Fix traffic counters for remote request handler processes

8 files changed, 106 insertions, 40 deletions

diff --git a/lib/diameter/doc/src/diameter.xml b/lib/diameter/doc/src/diameter.xml
index 85522c99b2..0ab6a122cd 100644
--- a/lib/diameter/doc/src/diameter.xml
+++ b/lib/diameter/doc/src/diameter.xml
@@ -23,7 +23,7 @@
 
 <copyright>
 <year>2011</year>
-<year>2019</year>
+<year>2020</year>
 <holder>Ericsson AB. All Rights Reserved.</holder>
 </copyright>
 <legalnotice>
@@ -1398,10 +1398,11 @@ Options <c>monitor</c> and <c>link</c> are ignored in the list-valued
 case.
 An MFA is applied with an additional term prepended to its argument
 list, and should return either the pid of the handler process that
-invokes <c>diameter_traffic:request/1</c> on the term in order to
+invokes <c>diameter_traffic:request/1</c> on the argument in order to
 process the request, or the atom <c>discard</c>.
-The handler process need not be local, but diameter must be started on
-the remote node.</p>
+The handler process need not be local, and diameter need not be
+started on the remote node, but diameter and relevant application
+callbacks must be on the code path.</p>
 
 <p>
 Defaults to the empty list.</p>
diff --git a/lib/diameter/src/Makefile b/lib/diameter/src/Makefile
index 98636ed6e2..2d6dcc811e 100644
--- a/lib/diameter/src/Makefile
+++ b/lib/diameter/src/Makefile
@@ -1,7 +1,7 @@
 #
 # %CopyrightBegin%
 #
-# Copyright Ericsson AB 2010-2018. All Rights Reserved.
+# Copyright Ericsson AB 2010-2020. All Rights Reserved.
 #
 # Licensed under the Apache License, Version 2.0 (the "License");
 # you may not use this file except in compliance with the License.
@@ -210,12 +210,16 @@ realclean: clean
 
 PLT = ./otp.plt
 
-plt:
+plt: $(PLT)
+
+$(PLT):
 	dialyzer --build_plt \
 	         --apps erts stdlib kernel \
 	                xmerl ssl public_key crypto \
 	                compiler syntax_tools runtime_tools \
-	         --output_plt $(PLT) \
+	         --output_plt $@ \
+	         --get_warnings \
+	         --statistics \
 	         --verbose
 
 dialyze: opt $(PLT)
diff --git a/lib/diameter/src/base/diameter.erl b/lib/diameter/src/base/diameter.erl
index 7f172e1fa1..ee5ff38a42 100644
--- a/lib/diameter/src/base/diameter.erl
+++ b/lib/diameter/src/base/diameter.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2019. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -146,8 +146,9 @@ services() ->
 %% service_info/2
 %% ---------------------------------------------------------------------------
 
--spec service_info(service_name(), atom() | [atom()])
-   -> any().
+-spec service_info(service_name(), Item | [Item])
+   -> any()
+ when Item :: atom() | peer_ref().
 
 service_info(SvcName, Option) ->
     diameter_service:info(SvcName, Option).
diff --git a/lib/diameter/src/base/diameter_dist.erl b/lib/diameter/src/base/diameter_dist.erl
index ed23152b8b..1edca58eb9 100644
--- a/lib/diameter/src/base/diameter_dist.erl
+++ b/lib/diameter/src/base/diameter_dist.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2019. All Rights Reserved.
+%% Copyright Ericsson AB 2019-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -27,6 +27,21 @@
 %% transport configuration, to be able to distribute incoming Diameter
 %% requests to handler processes (local or remote) in various ways.
 %%
+%% The gen_server implemented here must be started on each node on
+%% which {diameter_dist, route_session, _} is configured as a
+%% spawn_opt MFA, as well as each node that wants to receive requests.
+%% This happens as a consequence of diameter application start, but a
+%% minimal solution could start only this server on nodes that should
+%% handle requests but not configure transport. (Although the typical
+%% case is probably that diameter should be started in any case; for
+%% example, to be able to originate requests.)
+%%
+%% Moreover, attach/1 must be called to communicate the list of
+%% services for which the local node is willing to handle requests.
+%% The servers on different nodes communicate so that each server
+%% knows which nodes are prepared to handle requests for which
+%% services.
+%%
 
 %% spawn_opt callbacks
 -export([spawn_local/2,
diff --git a/lib/diameter/src/base/diameter_peer_fsm.erl b/lib/diameter/src/base/diameter_peer_fsm.erl
index cf5e7f21d3..b86dcaf923 100644
--- a/lib/diameter/src/base/diameter_peer_fsm.erl
+++ b/lib/diameter/src/base/diameter_peer_fsm.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2010-2018. All Rights Reserved.
+%% Copyright Ericsson AB 2010-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
diff --git a/lib/diameter/src/base/diameter_traffic.erl b/lib/diameter/src/base/diameter_traffic.erl
index 8423e30269..76f2420c7b 100644
--- a/lib/diameter/src/base/diameter_traffic.erl
+++ b/lib/diameter/src/base/diameter_traffic.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2013-2019. All Rights Reserved.
+%% Copyright Ericsson AB 2013-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -284,12 +284,18 @@ recv(false, false, TPid, Pkt, _, _) ->
 spawn_request(false, _, _, _, _, _, _) ->  %% no transport
     discard;
 
-%% An MFA should return the pid() of a process in which the argument
-%% fun in applied, or the atom 'discard' if the fun is not applied.
-%% The latter results in an acknowledgment back to the transport
-%% process when appropriate, to ensure that send/recv callbacks can
-%% count outstanding requests. Acknowledgement is implicit if the
-%% handler process dies (in a handle_request callback for example).
+%% An MFA should return the pid() of a process that invokes
+%% diameter_traffic:request(ReqT), or the atom 'discard' if the
+%% function is not called. The latter results in an acknowledgment
+%% back to the transport process when appropriate, to ensure that
+%% send/recv callbacks can count outstanding requests. Acknowledgement
+%% is implicit if the handler process dies (in a handle_request
+%% callback for example).
+%%
+%% There is no requirement that diameter be started on nodes on which
+%% handler processes are spawned, just that diameter and application
+%% callbacks are on the code path. (Although the MFA itself may have
+%% requirements, as in the case of diameter_dist.)
 spawn_request(AppT, {M,F,A}, Ack, TPid, Pkt, Dict0, RecvData) ->
     %% Term to pass to request/1 in an appropriate process. Module
     %% diameter_dist implements callbacks.
@@ -1239,7 +1245,12 @@ is_result(RC, true, _) ->
 %% incr/2
 
 incr(TPid, Counter) ->
-    diameter_stats:incr(Counter, TPid, 1).
+    Node = node(TPid),
+    if Node == node() ->
+            diameter_stats:incr(Counter, TPid, 1);
+       true ->
+            spawn(Node, diameter_stats, incr, [Counter, TPid, 1])
+    end.
 
 %% rcc/1
 
@@ -1866,23 +1877,26 @@ z(#diameter_packet{header = H, bin = Bin, transport_data = T}) ->
                      transport_data = T}.
 
 %% send/1
+%%
+%% Send from a remote node using a peer connection on this one. Pkt is
+%% already stripped.
 
 send({TPid, Pkt, #request{handler = Pid} = Req0, SvcName, Timeout, TRef}) ->
     Req = Req0#request{handler = self()},
-    recv(TPid, Pid, TRef, zend_requezt(TPid, Pkt, Req, SvcName, Timeout)).
+    Pid ! recv(TPid, TRef, send_request(TPid, Pkt, Req, SvcName, Timeout)).
 
-%% recv/4
+%% recv/3
 %%
 %% Relay an answer from a remote node.
 
-recv(TPid, Pid, TRef, {LocalTRef, MRef}) ->
+recv(TPid, TRef, {LocalTRef, MRef}) ->
     receive
         {answer, _, _, _, _} = A ->
-            Pid ! A;
+            A;
         {'DOWN', MRef, process, _, _} ->
-            Pid ! {failover, TRef};
+            {failover, TRef};
         {failover = T, LocalTRef} ->
-            Pid ! {T, TRef};
+            {T, TRef};
         T ->
             exit({timeout, LocalTRef, TPid} = T)
     end.
diff --git a/lib/diameter/src/transport/diameter_tcp.erl b/lib/diameter/src/transport/diameter_tcp.erl
index e5e766d2a0..a051aeb156 100644
--- a/lib/diameter/src/transport/diameter_tcp.erl
+++ b/lib/diameter/src/transport/diameter_tcp.erl
@@ -569,7 +569,11 @@ m({'DOWN', M, process, P, _} = T, #monitor{parent = MRef,
 
 %% l/2
 %%
-%% Transition listener state.
+%% Transition listener state. Or not anymore since any message causes
+%% the process to exit.
+
+-spec l(tuple(), #listener{})
+   -> no_return().
 
 %% Service process has died.
 l({'DOWN', _, process, Pid, _} = T, #listener{service = Pid,
diff --git a/lib/diameter/test/diameter_dist_SUITE.erl b/lib/diameter/test/diameter_dist_SUITE.erl
index b2e4c35b9a..2fda2830ae 100644
--- a/lib/diameter/test/diameter_dist_SUITE.erl
+++ b/lib/diameter/test/diameter_dist_SUITE.erl
@@ -1,7 +1,7 @@
 %%
 %% %CopyrightBegin%
 %%
-%% Copyright Ericsson AB 2019. All Rights Reserved.
+%% Copyright Ericsson AB 2019-2020. All Rights Reserved.
 %%
 %% Licensed under the Apache License, Version 2.0 (the "License");
 %% you may not use this file except in compliance with the License.
@@ -160,15 +160,32 @@ ping(Config) ->
 %%
 %% Start diameter services.
 
-start(SvcName)
-  when is_atom(SvcName) ->
+%% There's no need to start diameter on a node that only services
+%% diameter_dist as a handler of incoming requests, but the
+%% diameter_dist server must be started since the servers communicate
+%% to determine who services what. The typical case is probably that
+%% handler nodes also want to be able to send Diameter requests, in
+%% which case the application needs to be started and diameter_dist is
+%% started as a part of this, but only start the server here to ensure
+%% everything still works as expected.
+start({_SvcName, [_, {S1, _}, {S2, _}, _]}) 
+  when node() == S1;    %% server1
+       node() == S2 ->  %% server2
+    Mod = diameter_dist,
+    {ok, _} = gen_server:start({local, Mod}, Mod, _Args = [], _Opts  = []),
+    ok;
+
+start({SvcName, [{S0, _}, _, _, {C, _}]})
+  when node() == S0;    %% server0
+       node() == C ->   %% client
     ok = diameter:start(),
     ok = diameter:start_service(SvcName, ?SERVICE((?L(SvcName))));
 
-start(Config) ->
+start(Config)
+  when is_list(Config) ->
     Nodes = ?util:read_priv(Config, nodes),
     [] = [{N,RC} || {N,S} <- Nodes,
-                    RC <- [rpc:call(N, ?MODULE, start, [S])],
+                    RC <- [rpc:call(N, ?MODULE, start, [{S, Nodes}])],
                     RC /= ok].
 
 sequence() ->
@@ -194,13 +211,12 @@ origin(Server) ->
 %% Establish one connection from the client, terminated on the first
 %% server node, the others handling requests.
 
-connect({?SERVER, Config, [{Node, _} | _]}) ->
-    if Node == node() ->  %% server0
-            ?util:write_priv(Config, lref, {Node, ?util:listen(?SERVER, tcp)});
-       true ->
-            diameter_dist:attach([?SERVER])
-    end,
-    ok;
+connect({?SERVER, Config, [{Node, _} | _]})
+  when Node == node() ->  %% server0
+    ok = ?util:write_priv(Config, lref, {Node, ?util:listen(?SERVER, tcp)});
+
+connect({?SERVER, _Config, _}) -> %% server[12]: register to receive requests
+    ok = diameter_dist:attach([?SERVER]);
 
 connect({?CLIENT, Config, _}) ->
     ?util:connect(?CLIENT, tcp, ?util:read_priv(Config, lref)),
@@ -239,7 +255,18 @@ send(Config) ->
 send(Config, 0, Dict) ->
     [{Server0, _} | _] = ?util:read_priv(Config, nodes) ,
     Node = atom_to_binary(Server0, utf8),
-    {false, _} = {dict:is_key(Node, Dict), dict:to_list(Dict)};
+    {false, _} = {dict:is_key(Node, Dict), dict:to_list(Dict)},
+    %% Check that counters have been incremented as expected on server0.
+    [Info] = rpc:call(Server0, diameter, service_info, [?SERVER, connections]),
+    {[Stats], _} = {[S || {statistics, S} <- Info], Info},
+    {[{recv, 1, 100}, {send, 0, 100}], _}
+        = {[{D,R,N} || T <- [recv, send],
+                       {{{0,275,R}, D}, N} <- Stats,
+                       D == T],
+           Stats},
+    {[{send, 0, 100, 2001}], _}
+        = {[{D,R,N,C} || {{{0,275,R}, D, {'Result-Code', C}}, N} <- Stats],
+           Stats};
 
 send(Config, N, Dict) ->
     #diameter_base_STA{'Result-Code' = ?SUCCESS,