Page MenuHomePhorge

No OneTemporary

Authored By
Unknown
Size
5 KB
Referenced Files
None
Subscribers
None
diff --git a/changelog.d/gun_pool4.fix b/changelog.d/gun_pool4.fix
new file mode 100644
index 000000000..f68c1c970
--- /dev/null
+++ b/changelog.d/gun_pool4.fix
@@ -0,0 +1 @@
+Gun Connection Pool was not retrying to acquire a connection if the pool was full and stale connections were reclaimed
diff --git a/lib/pleroma/gun/connection_pool/worker_supervisor.ex b/lib/pleroma/gun/connection_pool/worker_supervisor.ex
index eb83962d8..dabb15b08 100644
--- a/lib/pleroma/gun/connection_pool/worker_supervisor.ex
+++ b/lib/pleroma/gun/connection_pool/worker_supervisor.ex
@@ -1,51 +1,61 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Gun.ConnectionPool.WorkerSupervisor do
@moduledoc "Supervisor for pool workers. Does not do anything except enforce max connection limit"
+ alias Pleroma.Config
+ alias Pleroma.Gun.ConnectionPool.Worker
+
use DynamicSupervisor
def start_link(opts) do
DynamicSupervisor.start_link(__MODULE__, opts, name: __MODULE__)
end
def init(_opts) do
DynamicSupervisor.init(
strategy: :one_for_one,
- max_children: Pleroma.Config.get([:connections_pool, :max_connections])
+ max_children: Config.get([:connections_pool, :max_connections])
)
end
- def start_worker(opts, last_attempt \\ false) do
- case DynamicSupervisor.start_child(__MODULE__, {Pleroma.Gun.ConnectionPool.Worker, opts}) do
+ def start_worker(opts, last_attempt \\ false)
+
+ def start_worker(opts, true) do
+ case DynamicSupervisor.start_child(__MODULE__, {Worker, opts}) do
+ {:error, :max_children} ->
+ :telemetry.execute([:pleroma, :connection_pool, :provision_failure], %{opts: opts})
+ {:error, :pool_full}
+
+ res ->
+ res
+ end
+ end
+
+ def start_worker(opts, false) do
+ case DynamicSupervisor.start_child(__MODULE__, {Worker, opts}) do
{:error, :max_children} ->
- funs = [fn -> last_attempt end, fn -> match?(:error, free_pool()) end]
-
- if Enum.any?(funs, fn fun -> fun.() end) do
- :telemetry.execute([:pleroma, :connection_pool, :provision_failure], %{opts: opts})
- {:error, :pool_full}
- else
- start_worker(opts, true)
- end
+ spawn(fn -> free_pool() end)
+ start_worker(opts, true)
res ->
res
end
end
defp free_pool do
wait_for_reclaimer_finish(Pleroma.Gun.ConnectionPool.Reclaimer.start_monitor())
end
defp wait_for_reclaimer_finish({pid, mon}) do
receive do
{:DOWN, ^mon, :process, ^pid, :no_unused_conns} ->
:error
{:DOWN, ^mon, :process, ^pid, :normal} ->
:ok
end
end
end
diff --git a/test/pleroma/gun/connection_pool_test.exs b/test/pleroma/gun/connection_pool_test.exs
index e0c9e9904..f3670760d 100644
--- a/test/pleroma/gun/connection_pool_test.exs
+++ b/test/pleroma/gun/connection_pool_test.exs
@@ -1,100 +1,99 @@
# Pleroma: A lightweight social networking server
# Copyright © 2017-2022 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only
defmodule Pleroma.Gun.ConnectionPoolTest do
use Pleroma.DataCase
import Mox
import ExUnit.CaptureLog
alias Pleroma.Gun.ConnectionPool
defp gun_mock(_) do
Pleroma.GunMock
|> stub(:open, fn _, _, _ -> Task.start_link(fn -> Process.sleep(100) end) end)
|> stub(:await_up, fn _, _ -> {:ok, :http} end)
|> stub(:set_owner, fn _, _ -> :ok end)
:ok
end
setup :gun_mock
test "gives the same connection to 2 concurrent requests" do
Enum.map(
[
"http://www.korean-books.com.kp/KBMbooks/en/periodic/pictorial/20200530163914.pdf",
"http://www.korean-books.com.kp/KBMbooks/en/periodic/pictorial/20200528183427.pdf"
],
fn uri ->
uri = URI.parse(uri)
task_parent = self()
Task.start_link(fn ->
{:ok, conn} = ConnectionPool.get_conn(uri, [])
ConnectionPool.release_conn(conn)
send(task_parent, conn)
end)
end
)
[pid, pid] =
for _ <- 1..2 do
receive do
pid -> pid
end
end
end
- @tag :erratic
test "connection limit is respected with concurrent requests" do
clear_config([:connections_pool, :max_connections]) do
clear_config([:connections_pool, :max_connections], 1)
# The supervisor needs a reboot to apply the new config setting
Process.exit(Process.whereis(Pleroma.Gun.ConnectionPool.WorkerSupervisor), :kill)
on_exit(fn ->
Process.exit(Process.whereis(Pleroma.Gun.ConnectionPool.WorkerSupervisor), :kill)
end)
end
capture_log(fn ->
Enum.map(
[
"https://ninenines.eu/",
"https://youtu.be/PFGwMiDJKNY"
],
fn uri ->
uri = URI.parse(uri)
task_parent = self()
Task.start_link(fn ->
result = ConnectionPool.get_conn(uri, [])
# Sleep so that we don't end up with a situation,
# where request from the second process gets processed
# only after the first process already released the connection
Process.sleep(50)
case result do
{:ok, pid} ->
ConnectionPool.release_conn(pid)
_ ->
nil
end
send(task_parent, result)
end)
end
)
[{:error, :pool_full}, {:ok, _pid}] =
for _ <- 1..2 do
receive do
result -> result
end
end
|> Enum.sort()
end)
end
end

File Metadata

Mime Type
text/x-diff
Expires
Thu, Oct 29, 1:25 PM (1 d, 3 h)
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
1798137
Default Alt Text
(5 KB)

Event Timeline