Use process labels (#330)

Author: nathanl

lib/db_connection/connection.ex

@@ -6,6 +6,7 @@ defmodule DBConnection.Connection do
   require Logger
   alias DBConnection.Backoff
   alias DBConnection.Holder
+  alias DBConnection.Util
 
   @timeout 15_000
 
@@ -47,6 +48,10 @@ defmodule DBConnection.Connection do
   @doc false
   @impl :gen_statem
   def init({mod, opts, pool, tag}) do
+    pool_index = Keyword.get(opts, :pool_index)
+    label = if pool_index, do: "db_conn_#{pool_index}", else: "db_conn"
+    Util.set_label(label)
+
     s = %{
       mod: mod,
       opts: opts,
@@ -267,15 +272,19 @@ defmodule DBConnection.Connection do
         :no_state,
         %{client: {ref, :after_connect}} = s
       ) do
-    message = "client #{inspect(pid)} exited: " <> Exception.format_exit(reason)
+    message =
+      "client #{Util.inspect_pid(pid)} exited: " <> Exception.format_exit(reason)
+
     err = DBConnection.ConnectionError.exception(message)
 
     {:keep_state, %{s | client: {nil, :after_connect}},
      {:next_event, :internal, {:disconnect, {down_log(reason), err}}}}
   end
 
   def handle_event(:info, {:DOWN, mon, _, pid, reason}, :no_state, %{client: {ref, mon}} = s) do
-    message = "client #{inspect(pid)} exited: " <> Exception.format_exit(reason)
+    message =
+      "client #{Util.inspect_pid(pid)} exited: " <> Exception.format_exit(reason)
+
     err = DBConnection.ConnectionError.exception(message)
 
     {:keep_state, %{s | client: {ref, nil}},
@@ -290,14 +299,14 @@ defmodule DBConnection.Connection do
       )
       when is_reference(timer) do
     message =
-      "client #{inspect(pid)} timed out because it checked out " <>
+      "client #{Util.inspect_pid(pid)} timed out because it checked out " <>
         "the connection for longer than #{timeout}ms"
 
     exc =
       case Process.info(pid, :current_stacktrace) do
         {:current_stacktrace, stacktrace} ->
           message <>
-            "\n\n#{inspect(pid)} was at location:\n\n" <>
+            "\n\n#{Util.inspect_pid(pid)} was at location:\n\n" <>
             Exception.format_stacktrace(stacktrace)
 
         _ ->

lib/db_connection/connection_pool.ex

@@ -15,6 +15,7 @@ defmodule DBConnection.ConnectionPool do
 
   use GenServer
   alias DBConnection.Holder
+  alias DBConnection.Util
 
   @behaviour DBConnection.Pool
 
@@ -136,7 +137,7 @@ defmodule DBConnection.ConnectionPool do
   end
 
   def handle_info({:"ETS-TRANSFER", holder, pid, queue}, {_, queue, _, _} = data) do
-    message = "client #{inspect(pid)} exited"
+    message = "client #{Util.inspect_pid(pid)} exited"
     err = DBConnection.ConnectionError.exception(message: message, severity: :info)
     Holder.handle_disconnect(holder, err)
     {:noreply, data}
@@ -175,14 +176,14 @@ defmodule DBConnection.ConnectionPool do
     # Check that timeout refers to current holder (and not previous)
     if Holder.handle_deadline(holder, deadline) do
       message =
-        "client #{inspect(pid)} timed out because " <>
+        "client #{Util.inspect_pid(pid)} timed out because " <>
           "it queued and checked out the connection for longer than #{len}ms"
 
       exc =
         case Process.info(pid, :current_stacktrace) do
           {:current_stacktrace, stacktrace} ->
             message <>
-              "\n\n#{inspect(pid)} was at location:\n\n" <>
+              "\n\n#{Util.inspect_pid(pid)} was at location:\n\n" <>
               Exception.format_stacktrace(stacktrace)
 
           _ ->

lib/db_connection/ownership/manager.ex

@@ -3,6 +3,7 @@ defmodule DBConnection.Ownership.Manager do
   use GenServer
   require Logger
   alias DBConnection.Ownership.Proxy
+  alias DBConnection.Util
 
   @timeout 5_000
 
@@ -394,7 +395,7 @@ defmodule DBConnection.Ownership.Manager do
 
   defp not_found({pid, _} = from) do
     msg = """
-    cannot find ownership process for #{inspect(pid)}.
+    cannot find ownership process for #{Util.inspect_pid(pid)}.
 
     When using ownership, you must manage connections in one
     of the four ways:

lib/db_connection/ownership/proxy.ex

@@ -2,6 +2,7 @@ defmodule DBConnection.Ownership.Proxy do
   @moduledoc false
 
   alias DBConnection.Holder
+  alias DBConnection.Util
   use GenServer, restart: :temporary
 
   @time_unit 1000
@@ -21,6 +22,8 @@ defmodule DBConnection.Ownership.Proxy do
 
   @impl true
   def init({caller, pool, pool_opts}) do
+    Util.set_label("db_ownership_proxy")
+
     pool_opts =
       pool_opts
       |> Keyword.put(:timeout, :infinity)
@@ -58,13 +61,13 @@ defmodule DBConnection.Ownership.Proxy do
 
   @impl true
   def handle_info({:DOWN, ref, _, pid, _reason}, %{owner: {_, ref}} = state) do
-    down("owner #{inspect(pid)} exited", state)
+    down("owner #{Util.inspect_pid(pid)} exited", state)
   end
 
   def handle_info({:timeout, deadline, {_ref, holder, pid, len}}, %{holder: holder} = state) do
     if Holder.handle_deadline(holder, deadline) do
       message =
-        "client #{inspect(pid)} timed out because " <>
+        "client #{Util.inspect_pid(pid)} timed out because " <>
           "it queued and checked out the connection for longer than #{len}ms"
 
       down(message, state)
@@ -78,7 +81,7 @@ defmodule DBConnection.Ownership.Proxy do
         %{ownership_timer: timer} = state
       ) do
     message =
-      "owner #{inspect(pid)} timed out because " <>
+      "owner #{Util.inspect_pid(pid)} timed out because " <>
         "it owned the connection for longer than #{timeout}ms (set via the :ownership_timeout option)"
 
     # We don't invoke down because this is always a disconnect, even if there is no client.
@@ -150,7 +153,7 @@ defmodule DBConnection.Ownership.Proxy do
   end
 
   def handle_info({:"ETS-TRANSFER", holder, pid, ref}, %{holder: holder, owner: {_, ref}} = state) do
-    down("client #{inspect(pid)} exited", state)
+    down("client #{Util.inspect_pid(pid)} exited", state)
   end
 
   @impl true

lib/db_connection/task.ex

@@ -13,6 +13,8 @@ defmodule DBConnection.Task do
   end
 
   def init(fun, parent, opts) do
+    DBConnection.Util.set_label("db_after_connect_task")
+
     try do
       Process.link(parent)
     catch

lib/db_connection/util.ex

@@ -0,0 +1,52 @@
+defmodule DBConnection.Util do
+  @moduledoc false
+
+  @doc """
+  Inspect a pid, including the process label if possible.
+  """
+  def inspect_pid(pid) do
+    with :undefined <- get_label(pid),
+         :undefined <- get_name(pid) do
+      inspect(pid)
+    else
+      label_or_name -> "#{inspect(pid)} (#{inspect(label_or_name)})"
+    end
+  end
+
+  defp get_name(pid) do
+    try do
+      Process.info(pid, :registered_name)
+    rescue
+      _ -> :undefined
+    else
+      {:registered_name, name} when is_atom(name) -> name
+      _ -> :undefined
+    end
+  end
+
+  @doc """
+  Set a process label if `Process.set_label/1` is available.
+  """
+  def set_label(label) do
+    if function_exported?(:proc_lib, :set_label, 1) do
+      :proc_lib.set_label(label)
+    else
+      :ok
+    end
+  end
+
+  # Get a process label if `:proc_lib.get_label/1` is available.
+  defp get_label(pid) do
+    if function_exported?(:proc_lib, :get_label, 1) do
+      # Avoid a compiler warning if the function isn't
+      # defined in your version of Erlang/OTP
+      apply(:proc_lib, :get_label, [pid])
+    else
+      # mimic return value of
+      # `:proc_lib.get_label/1` when none is set.
+      # Don't resort to using `Process.info(pid, :dictionary)`,
+      # as this is not efficient.
+      :undefined
+    end
+  end
+end