Skip to content

Commit

Permalink
HTTP: radically simplify pool checkin/checkout
Browse files Browse the repository at this point in the history
Use a custom tesla middleware instead of adapter helper function +
custom redirect middleware.

This will also fix "Client died before releasing the connection"
messages when the request pool is overloaded. Since the checkout is
now done after passing ConcurrentLimiter.

This is technically less efficient, since the connection needs to be
checked in/out every time the middleware is left or entered respectively.
But I don't think the nanoseconds we might lose on redirects
to the same host are worth the complexity.
  • Loading branch information
rinpatch committed Sep 3, 2020
1 parent 9433311 commit d34fe28
Show file tree
Hide file tree
Showing 6 changed files with 51 additions and 149 deletions.
4 changes: 0 additions & 4 deletions lib/pleroma/http/adapter_helper.ex
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ defmodule Pleroma.HTTP.AdapterHelper do
| {Connection.proxy_type(), Connection.host(), pos_integer()}

@callback options(keyword(), URI.t()) :: keyword()
@callback get_conn(URI.t(), keyword()) :: {:ok, term()} | {:error, term()}

@spec format_proxy(String.t() | tuple() | nil) :: proxy() | nil
def format_proxy(nil), do: nil
Expand Down Expand Up @@ -47,9 +46,6 @@ defmodule Pleroma.HTTP.AdapterHelper do
|> adapter_helper().options(uri)
end

@spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} | {:error, atom()}
def get_conn(uri, opts), do: adapter_helper().get_conn(uri, opts)

defp adapter, do: Application.get_env(:tesla, :adapter)

defp adapter_helper do
Expand Down
9 changes: 0 additions & 9 deletions lib/pleroma/http/adapter_helper/gun.ex
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do
@behaviour Pleroma.HTTP.AdapterHelper

alias Pleroma.Config
alias Pleroma.Gun.ConnectionPool
alias Pleroma.HTTP.AdapterHelper

require Logger
Expand Down Expand Up @@ -57,14 +56,6 @@ defmodule Pleroma.HTTP.AdapterHelper.Gun do
Config.get([:pools, pool, :timeout], default)
end

@spec get_conn(URI.t(), keyword()) :: {:ok, keyword()} | {:error, atom()}
def get_conn(uri, opts) do
case ConnectionPool.get_conn(uri, opts) do
{:ok, conn_pid} -> {:ok, Keyword.merge(opts, conn: conn_pid, close_conn: false)}
err -> err
end
end

@prefix Pleroma.Gun.ConnectionPool
def limiter_setup do
wait = Config.get([:connections_pool, :connection_acquisition_wait])
Expand Down
3 changes: 0 additions & 3 deletions lib/pleroma/http/adapter_helper/hackney.ex
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,4 @@ defmodule Pleroma.HTTP.AdapterHelper.Hackney do
end

defp add_scheme_opts(opts, _), do: opts

@spec get_conn(URI.t(), keyword()) :: {:ok, keyword()}
def get_conn(_uri, opts), do: {:ok, opts}
end
39 changes: 16 additions & 23 deletions lib/pleroma/http/http.ex
Original file line number Diff line number Diff line change
Expand Up @@ -62,28 +62,21 @@ defmodule Pleroma.HTTP do
uri = URI.parse(url)
adapter_opts = AdapterHelper.options(uri, options[:adapter] || [])

case AdapterHelper.get_conn(uri, adapter_opts) do
{:ok, adapter_opts} ->
options = put_in(options[:adapter], adapter_opts)
params = options[:params] || []
request = build_request(method, headers, options, url, body, params)

adapter = Application.get_env(:tesla, :adapter)

client = Tesla.client(adapter_middlewares(adapter), adapter)

maybe_limit(
fn ->
request(client, request)
end,
adapter,
adapter_opts
)

# Connection release is handled in a custom FollowRedirects middleware
err ->
err
end
options = put_in(options[:adapter], adapter_opts)
params = options[:params] || []
request = build_request(method, headers, options, url, body, params)

adapter = Application.get_env(:tesla, :adapter)

client = Tesla.client(adapter_middlewares(adapter), adapter)

maybe_limit(
fn ->
request(client, request)
end,
adapter,
adapter_opts
)
end

@spec request(Client.t(), keyword()) :: {:ok, Env.t()} | {:error, any()}
Expand All @@ -110,7 +103,7 @@ defmodule Pleroma.HTTP do
end

defp adapter_middlewares(Tesla.Adapter.Gun) do
[Pleroma.HTTP.Middleware.FollowRedirects]
[Tesla.Middleware.FollowRedirects, Pleroma.Tesla.Middleware.ConnectionPool]
end

defp adapter_middlewares(_), do: []
Expand Down
35 changes: 35 additions & 0 deletions lib/pleroma/tesla/middleware/connection_pool.ex
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
# Pleroma: A lightweight social networking server
# Copyright © 2020 Pleroma Authors <https://pleroma.social/>
# SPDX-License-Identifier: AGPL-3.0-only

defmodule Pleroma.Tesla.Middleware.ConnectionPool do
@moduledoc """
Middleware to get/release connections from `Pleroma.Gun.ConnectionPool`
"""

@behaviour Tesla.Middleware

alias Pleroma.Gun.ConnectionPool

@impl Tesla.Middleware
def call(%Tesla.Env{url: url, opts: opts} = env, next, _) do
uri = URI.parse(url)

case ConnectionPool.get_conn(uri, opts[:adapter]) do
{:ok, conn_pid} ->
adapter_opts = Keyword.merge(opts[:adapter], conn: conn_pid, close_conn: false)
opts = Keyword.put(opts, :adapter, adapter_opts)
env = %{env | opts: opts}
res = Tesla.run(env, next)

unless opts[:adapter][:body_as] == :chunks do
ConnectionPool.release_conn(conn_pid)
end

res

err ->
err
end
end
end
110 changes: 0 additions & 110 deletions lib/pleroma/tesla/middleware/follow_redirects.ex

This file was deleted.

0 comments on commit d34fe28

Please sign in to comment.