Merge branch 'v2' into fix/always-monitor-manager

flake.nix

@@ -71,6 +71,7 @@
               ];
 
               pre-commit.hooks = {
+                mix-format.enable = true;
                 # credo.enable = true;
               };
 

lib/supavisor/monitoring/prom_ex.ex

@@ -29,7 +29,7 @@ defmodule Supavisor.Monitoring.PromEx do
     ]
   end
 
-  @spec remove_metrics(S.id()) :: non_neg_integer
+  @spec remove_metrics(S.id()) :: :ok
   def remove_metrics({{type, tenant}, user, mode, db_name, search_path} = id) do
     Logger.debug("Removing metrics for #{inspect(id)}")
 
@@ -42,11 +42,16 @@ defmodule Supavisor.Monitoring.PromEx do
       search_path: search_path
     }
 
-    Supavisor.Monitoring.PromEx.Metrics
-    |> :ets.select_delete([
-      {{{:_, meta}, :_}, [], [true]},
-      {{{:_, meta, :_}, :_}, [], [true]}
-    ])
+    {_, tids} = Peep.Persistent.storage(Supavisor.Monitoring.PromEx.Metrics)
+
+    tids
+    |> List.wrap()
+    |> Enum.each(
+      &:ets.select_delete(&1, [
+        {{{:_, meta}, :_}, [], [true]},
+        {{{:_, meta, :_}, :_}, [], [true]}
+      ])
+    )
   end
 
   @spec set_metrics_tags() :: map()

test/integration/proxy_test.exs

@@ -54,7 +54,6 @@ defmodule Supavisor.Integration.ProxyTest do
   end
 
   test "the wrong password" do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
 
     url =
@@ -156,7 +155,6 @@ defmodule Supavisor.Integration.ProxyTest do
   end
 
   test "too many clients in session mode" do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
     port = Application.get_env(:supavisor, :proxy_port_session)
 
@@ -166,8 +164,8 @@ defmodule Supavisor.Integration.ProxyTest do
         port: port
       )
 
-    {:ok, pid1} = single_connection(connection_opts)
-    {:ok, pid2} = single_connection(connection_opts)
+    assert {:ok, _} = single_connection(connection_opts)
+    assert {:ok, _} = single_connection(connection_opts)
 
     :timer.sleep(1000)
 
@@ -182,8 +180,6 @@ defmodule Supavisor.Integration.ProxyTest do
                 pg_code: "XX000"
               }
             }} = single_connection(connection_opts)
-
-    for pid <- [pid1, pid2], do: :gen_statem.stop(pid)
   end
 
   test "http to proxy server returns 200 OK" do
@@ -219,7 +215,6 @@ defmodule Supavisor.Integration.ProxyTest do
   end
 
   test "limit client connections" do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
 
     connection_opts = [
@@ -243,7 +238,6 @@ defmodule Supavisor.Integration.ProxyTest do
   end
 
   test "change role password", %{origin: origin} do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
 
     conn = fn pass ->
@@ -273,7 +267,6 @@ defmodule Supavisor.Integration.ProxyTest do
   end
 
   test "invalid characters in user or db_name" do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
 
     url =
@@ -291,62 +284,7 @@ defmodule Supavisor.Integration.ProxyTest do
             }} = parse_uri(url) |> single_connection()
   end
 
-  # test "max_pools limit" do
-  #   Process.flag(:trap_exit, true)
-  #   db_conf = Application.get_env(:supavisor, Repo)
-  #   port = Application.get_env(:supavisor, :proxy_port_transaction)
-
-  #   tenant = "max_pool_tenant"
-
-  #   {:ok, pid1} =
-  #     Keyword.merge(db_conf,
-  #       username: "postgres.#{tenant}",
-  #       port: port
-  #     )
-  #     |> single_connection()
-
-  #   assert Supavisor.count_pools(tenant) == 1
-
-  #   {:ok, pid2} =
-  #     Keyword.merge(db_conf,
-  #       username: "session.#{tenant}",
-  #       port: port
-  #     )
-  #     |> single_connection()
-
-  #   assert Supavisor.count_pools(tenant) == 2
-
-  #   {:ok, pid3} =
-  #     Keyword.merge(db_conf,
-  #       username: "transaction.#{tenant}",
-  #       port: port
-  #     )
-  #     |> single_connection()
-
-  #   assert Supavisor.count_pools(tenant) == 3
-
-  #   connection_opts =
-  #     Keyword.merge(db_conf,
-  #       username: "max_clients.#{tenant}",
-  #       port: port
-  #     )
-
-  #   assert {:error,
-  #           %Postgrex.Error{
-  #             postgres: %{
-  #               code: :internal_error,
-  #               message: "Max pools count reached",
-  #               unknown: "FATAL",
-  #               severity: "FATAL",
-  #               pg_code: "XX000"
-  #             }
-  #           }} = single_connection(connection_opts)
-
-  #   for pid <- [pid1, pid2, pid3], do: :gen_statem.stop(pid)
-  # end
-
   test "active_count doesn't block" do
-    Process.flag(:trap_exit, true)
     db_conf = Application.get_env(:supavisor, Repo)
     port = Application.get_env(:supavisor, :proxy_port_session)
 
@@ -379,15 +317,18 @@ defmodule Supavisor.Integration.ProxyTest do
   defp single_connection(db_conf, c_port \\ nil) when is_list(db_conf) do
     port = c_port || db_conf[:port]
 
-    [
+    opts = [
       hostname: db_conf[:hostname],
       port: port,
       database: db_conf[:database],
       password: db_conf[:password],
       username: db_conf[:username],
       pool_size: 1
     ]
-    |> SingleConnection.connect()
+
+    with {:error, {error, _}} <- start_supervised({SingleConnection, opts}) do
+      {:error, error}
+    end
   end
 
   defp parse_uri(uri) do

test/supavisor/prom_ex_test.exs

@@ -1,19 +1,12 @@
 defmodule Supavisor.PromExTest do
-  use ExUnit.Case, async: true
-  use Supavisor.DataCase
+  use Supavisor.DataCase, async: true
 
   import Supavisor.Asserts
 
-  alias Supavisor.Monitoring.PromEx
+  @subject Supavisor.Monitoring.PromEx
 
   @tenant "prom_tenant"
 
-  # These tests are known to be flaky, and while these do not affect users
-  # directly we can run them independently when needed. In future we probably
-  # should make them pass "regularly", but for now that is easier to filter them
-  # out.
-  @moduletag flaky: true
-
   setup do
     db_conf = Application.get_env(:supavisor, Repo)
 
@@ -34,12 +27,14 @@ defmodule Supavisor.PromExTest do
   end
 
   test "remove tenant tag upon termination", %{proxy: proxy, user: user, db_name: db_name} do
-    assert PromEx.get_metrics() =~ "tenant=\"#{@tenant}\""
+    assert @subject.get_metrics() =~ "tenant=\"#{@tenant}\""
 
     :ok = GenServer.stop(proxy)
     :ok = Supavisor.stop({{:single, @tenant}, user, :transaction, db_name, nil})
 
-    refute_eventually(10, fn -> PromEx.get_metrics() =~ "tenant=\"#{@tenant}\"" end)
+    Process.sleep(1000)
+
+    refute_eventually(10, fn -> @subject.get_metrics() =~ "tenant=\"#{@tenant}\"" end)
   end
 
   test "clean_string/1 removes extra spaces from metric string" do
@@ -49,6 +44,6 @@ defmodule Supavisor.PromExTest do
     expected_output =
       "db_name=\"postgres\",mode=\"transaction\",tenant=\"dev_tenant\",type=\"single\",user=\"postgres\""
 
-    assert expected_output == PromEx.clean_string(input)
+    assert expected_output == @subject.clean_string(input)
   end
 end

test/support/asserts.ex

@@ -1,4 +1,8 @@
 defmodule Supavisor.Asserts do
+  @moduledoc """
+  Additional assertions useful in Supavisor tests
+  """
+
   @doc """
   Asserts that `function` will eventually success. Fails otherwise.
 

test/support/fixtures/single_connection.ex

@@ -5,6 +5,14 @@ defmodule SingleConnection do
 
   @behaviour P.SimpleConnection
 
+  def child_spec(conf) do
+    %{
+      id: {__MODULE__, System.unique_integer()},
+      start: {__MODULE__, :connect, [conf]},
+      restart: :temporary
+    }
+  end
+
   def connect(conf) do
     P.SimpleConnection.start_link(__MODULE__, [], conf)
   end