logo

pleroma

My custom branche(s) on git.pleroma.social/pleroma/pleroma
commit: 8180de951e2ac1efc6cfb9f64d078d6a1b7d0c0e
parent: d5d91ae689e14103551dd3622e208ea31e40c858
Author: kaniini <nenolod@gmail.com>
Date:   Sun,  3 Feb 2019 15:47:55 +0000

Merge branch '582_federation_retirement_optimization' into 'develop'

[#582] Optimized federation retirement by reducing the number of SQL calls

Closes #582

See merge request pleroma/pleroma!762

Diffstat:

Mlib/pleroma/instances/instance.ex25++++++++++++++++++-------
Mlib/pleroma/web/activity_pub/activity_pub.ex15+++++++++------
Mlib/pleroma/web/federator/federator.ex4++--
Mlib/pleroma/web/salmon/salmon.ex30+++++++++++++++++++-----------
Mlib/pleroma/web/websub/websub.ex14+++++++++-----
Mtest/web/activity_pub/activity_pub_test.exs73+++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++--
Mtest/web/federator_test.exs37+++++++++++++++++++++++++++++--------
Mtest/web/instances/instances_test.exs23+++++++++++++++++++++--
8 files changed, 178 insertions(+), 43 deletions(-)

diff --git a/lib/pleroma/instances/instance.ex b/lib/pleroma/instances/instance.ex @@ -26,7 +26,7 @@ defmodule Pleroma.Instances.Instance do |> unique_constraint(:host) end - def filter_reachable([]), do: [] + def filter_reachable([]), do: %{} def filter_reachable(urls_or_hosts) when is_list(urls_or_hosts) do hosts = @@ -34,17 +34,28 @@ defmodule Pleroma.Instances.Instance do |> Enum.map(&(&1 && host(&1))) |> Enum.filter(&(to_string(&1) != "")) - unreachable_hosts = + unreachable_since_by_host = Repo.all( from(i in Instance, - where: - i.host in ^hosts and - i.unreachable_since <= ^Instances.reachability_datetime_threshold(), - select: i.host + where: i.host in ^hosts, + select: {i.host, i.unreachable_since} ) ) + |> Map.new(& &1) - Enum.filter(urls_or_hosts, &(&1 && host(&1) not in unreachable_hosts)) + reachability_datetime_threshold = Instances.reachability_datetime_threshold() + + for entry <- Enum.filter(urls_or_hosts, &is_binary/1) do + host = host(entry) + unreachable_since = unreachable_since_by_host[host] + + if !unreachable_since || + NaiveDateTime.compare(unreachable_since, reachability_datetime_threshold) == :gt do + {entry, unreachable_since} + end + end + |> Enum.filter(& &1) + |> Map.new(& &1) end def reachable?(url_or_host) when is_binary(url_or_host) do diff --git a/lib/pleroma/web/activity_pub/activity_pub.ex b/lib/pleroma/web/activity_pub/activity_pub.ex @@ -744,7 +744,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do public = is_public?(activity) - remote_inboxes = + reachable_inboxes_metadata = (Pleroma.Web.Salmon.remote_users(activity) ++ remote_followers) |> Enum.filter(fn user -> User.ap_enabled?(user) end) |> Enum.map(fn %{info: %{source_data: data}} -> @@ -757,17 +757,18 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do {:ok, data} = Transmogrifier.prepare_outgoing(activity.data) json = Jason.encode!(data) - Enum.each(remote_inboxes, fn inbox -> + Enum.each(reachable_inboxes_metadata, fn {inbox, unreachable_since} -> Federator.enqueue(:publish_single_ap, %{ inbox: inbox, json: json, actor: actor, - id: activity.data["id"] + id: activity.data["id"], + unreachable_since: unreachable_since }) end) end - def publish_one(%{inbox: inbox, json: json, actor: actor, id: id}) do + def publish_one(%{inbox: inbox, json: json, actor: actor, id: id} = params) do Logger.info("Federating #{id} to #{inbox}") host = URI.parse(inbox).host @@ -791,11 +792,13 @@ defmodule Pleroma.Web.ActivityPub.ActivityPub do {"digest", digest} ] ) do - Instances.set_reachable(inbox) + if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since], + do: Instances.set_reachable(inbox) + result else {_post_result, response} -> - Instances.set_unreachable(inbox) + unless params[:unreachable_since], do: Instances.set_unreachable(inbox) {:error, response} end end diff --git a/lib/pleroma/web/federator/federator.ex b/lib/pleroma/web/federator/federator.ex @@ -124,8 +124,8 @@ defmodule Pleroma.Web.Federator do end end - def handle(:publish_single_salmon, {user_or_url, feed, poster}) do - Salmon.send_to_user(user_or_url, feed, poster) + def handle(:publish_single_salmon, params) do + Salmon.send_to_user(params) end def handle(:publish_single_ap, params) do diff --git a/lib/pleroma/web/salmon/salmon.ex b/lib/pleroma/web/salmon/salmon.ex @@ -162,30 +162,31 @@ defmodule Pleroma.Web.Salmon do |> Enum.filter(fn user -> user && !user.local end) end - # push an activity to remote accounts - # - def send_to_user(%{info: %{salmon: salmon}}, feed, poster), - do: send_to_user(salmon, feed, poster) + @doc "Pushes an activity to remote account." + def send_to_user(%{recipient: %{info: %{salmon: salmon}}} = params), + do: send_to_user(Map.put(params, :recipient, salmon)) - def send_to_user(url, feed, poster) when is_binary(url) do + def send_to_user(%{recipient: url, feed: feed, poster: poster} = params) when is_binary(url) do with {:ok, %{status: code}} when code in 200..299 <- poster.( url, feed, [{"Content-Type", "application/magic-envelope+xml"}] ) do - Instances.set_reachable(url) + if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since], + do: Instances.set_reachable(url) + Logger.debug(fn -> "Pushed to #{url}, code #{code}" end) :ok else e -> - Instances.set_unreachable(url) + unless params[:unreachable_since], do: Instances.set_reachable(url) Logger.debug(fn -> "Pushing Salmon to #{url} failed, #{inspect(e)}" end) :error end end - def send_to_user(_, _, _), do: :noop + def send_to_user(_), do: :noop @supported_activities [ "Create", @@ -218,13 +219,20 @@ defmodule Pleroma.Web.Salmon do remote_users = remote_users(activity) salmon_urls = Enum.map(remote_users, & &1.info.salmon) - reachable_salmon_urls = Instances.filter_reachable(salmon_urls) + reachable_urls_metadata = Instances.filter_reachable(salmon_urls) + reachable_urls = Map.keys(reachable_urls_metadata) remote_users - |> Enum.filter(&(&1.info.salmon in reachable_salmon_urls)) + |> Enum.filter(&(&1.info.salmon in reachable_urls)) |> Enum.each(fn remote_user -> Logger.debug(fn -> "Sending Salmon to #{remote_user.ap_id}" end) - Pleroma.Web.Federator.enqueue(:publish_single_salmon, {remote_user, feed, poster}) + + Pleroma.Web.Federator.enqueue(:publish_single_salmon, %{ + recipient: remote_user, + feed: feed, + poster: poster, + unreachable_since: reachable_urls_metadata[remote_user.info.salmon] + }) end) end end diff --git a/lib/pleroma/web/websub/websub.ex b/lib/pleroma/web/websub/websub.ex @@ -70,7 +70,8 @@ defmodule Pleroma.Web.Websub do subscriptions = Repo.all(query) callbacks = Enum.map(subscriptions, & &1.callback) - reachable_callbacks = Instances.filter_reachable(callbacks) + reachable_callbacks_metadata = Instances.filter_reachable(callbacks) + reachable_callbacks = Map.keys(reachable_callbacks_metadata) subscriptions |> Enum.filter(&(&1.callback in reachable_callbacks)) @@ -79,7 +80,8 @@ defmodule Pleroma.Web.Websub do xml: response, topic: topic, callback: sub.callback, - secret: sub.secret + secret: sub.secret, + unreachable_since: reachable_callbacks_metadata[sub.callback] } Pleroma.Web.Federator.enqueue(:publish_single_websub, data) @@ -268,7 +270,7 @@ defmodule Pleroma.Web.Websub do end) end - def publish_one(%{xml: xml, topic: topic, callback: callback, secret: secret}) do + def publish_one(%{xml: xml, topic: topic, callback: callback, secret: secret} = params) do signature = sign(secret || "", xml) Logger.info(fn -> "Pushing #{topic} to #{callback}" end) @@ -281,12 +283,14 @@ defmodule Pleroma.Web.Websub do {"X-Hub-Signature", "sha1=#{signature}"} ] ) do - Instances.set_reachable(callback) + if !Map.has_key?(params, :unreachable_since) || params[:unreachable_since], + do: Instances.set_reachable(callback) + Logger.info(fn -> "Pushed to #{callback}, code #{code}" end) {:ok, code} else {_post_result, response} -> - Instances.set_unreachable(callback) + unless params[:unreachable_since], do: Instances.set_reachable(callback) Logger.debug(fn -> "Couldn't push to #{callback}, #{inspect(response)}" end) {:error, response} end diff --git a/test/web/activity_pub/activity_pub_test.exs b/test/web/activity_pub/activity_pub_test.exs @@ -698,7 +698,57 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do end describe "publish_one/1" do - test_with_mock "it calls `Instances.set_unreachable` on target inbox on non-2xx HTTP response code", + test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is not specified", + Instances, + [:passthrough], + [] do + actor = insert(:user) + inbox = "http://200.site/users/nick1/inbox" + + assert {:ok, _} = ActivityPub.publish_one(%{inbox: inbox, json: "{}", actor: actor, id: 1}) + + assert called(Instances.set_reachable(inbox)) + end + + test_with_mock "calls `Instances.set_reachable` on successful federation if `unreachable_since` is set", + Instances, + [:passthrough], + [] do + actor = insert(:user) + inbox = "http://200.site/users/nick1/inbox" + + assert {:ok, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: NaiveDateTime.utc_now() + }) + + assert called(Instances.set_reachable(inbox)) + end + + test_with_mock "does NOT call `Instances.set_reachable` on successful federation if `unreachable_since` is nil", + Instances, + [:passthrough], + [] do + actor = insert(:user) + inbox = "http://200.site/users/nick1/inbox" + + assert {:ok, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: nil + }) + + refute called(Instances.set_reachable(inbox)) + end + + test_with_mock "calls `Instances.set_unreachable` on target inbox on non-2xx HTTP response code", Instances, [:passthrough], [] do @@ -724,7 +774,7 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do assert called(Instances.set_unreachable(inbox)) end - test_with_mock "it does NOT call `Instances.set_unreachable` if target is reachable", + test_with_mock "does NOT call `Instances.set_unreachable` if target is reachable", Instances, [:passthrough], [] do @@ -735,6 +785,25 @@ defmodule Pleroma.Web.ActivityPub.ActivityPubTest do refute called(Instances.set_unreachable(inbox)) end + + test_with_mock "does NOT call `Instances.set_unreachable` if target instance has non-nil `unreachable_since`", + Instances, + [:passthrough], + [] do + actor = insert(:user) + inbox = "http://connrefused.site/users/nick1/inbox" + + assert {:error, _} = + ActivityPub.publish_one(%{ + inbox: inbox, + json: "{}", + actor: actor, + id: 1, + unreachable_since: NaiveDateTime.utc_now() + }) + + refute called(Instances.set_unreachable(inbox)) + end end def data_uri do diff --git a/test/web/federator_test.exs b/test/web/federator_test.exs @@ -95,15 +95,18 @@ defmodule Pleroma.Web.FederatorTest do info: %{ap_enabled: true, source_data: %{"inbox" => inbox2}} }) - Instances.set_unreachable( - URI.parse(inbox2).host, - Instances.reachability_datetime_threshold() - ) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(inbox1, dt) + + Instances.set_consistently_unreachable(URI.parse(inbox2).host) {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"}) - assert called(Federator.enqueue(:publish_single_ap, %{inbox: inbox1})) + assert called( + Federator.enqueue(:publish_single_ap, %{inbox: inbox1, unreachable_since: dt}) + ) + refute called(Federator.enqueue(:publish_single_ap, %{inbox: inbox2})) end @@ -128,11 +131,20 @@ defmodule Pleroma.Web.FederatorTest do callback: "https://pleroma2.soykaf.com/cb" }) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(sub2.callback, dt) + Instances.set_consistently_unreachable(sub1.callback) {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI"}) - assert called(Federator.enqueue(:publish_single_websub, %{callback: sub2.callback})) + assert called( + Federator.enqueue(:publish_single_websub, %{ + callback: sub2.callback, + unreachable_since: dt + }) + ) + refute called(Federator.enqueue(:publish_single_websub, %{callback: sub1.callback})) end @@ -158,13 +170,22 @@ defmodule Pleroma.Web.FederatorTest do info: %{salmon: "https://domain2.com/salmon"} }) + dt = NaiveDateTime.utc_now() + Instances.set_unreachable(remote_user2.ap_id, dt) + Instances.set_consistently_unreachable("domain.com") {:ok, _activity} = CommonAPI.post(user, %{"status" => "HI @nick1@domain.com, @nick2@domain2.com!"}) - assert called(Federator.enqueue(:publish_single_salmon, {remote_user2, :_, :_})) - refute called(Federator.enqueue(:publish_single_websub, {remote_user1, :_, :_})) + assert called( + Federator.enqueue(:publish_single_salmon, %{ + recipient: remote_user2, + unreachable_since: dt + }) + ) + + refute called(Federator.enqueue(:publish_single_websub, %{recipient: remote_user1})) end end diff --git a/test/web/instances/instances_test.exs b/test/web/instances/instances_test.exs @@ -47,7 +47,7 @@ defmodule Pleroma.InstancesTest do end describe "filter_reachable/1" do - test "keeps only reachable elements of supplied list" do + setup do host = "consistently-unreachable.name" url1 = "http://eventually-unreachable.com/path" url2 = "http://domain.com/path" @@ -55,7 +55,26 @@ defmodule Pleroma.InstancesTest do Instances.set_consistently_unreachable(host) Instances.set_unreachable(url1) - assert [url1, url2] == Instances.filter_reachable([host, url1, url2]) + result = Instances.filter_reachable([host, url1, url2, nil]) + %{result: result, url1: url1, url2: url2} + end + + test "returns a map with keys containing 'not marked consistently unreachable' elements of supplied list", + %{result: result, url1: url1, url2: url2} do + assert is_map(result) + assert Enum.sort([url1, url2]) == result |> Map.keys() |> Enum.sort() + end + + test "returns a map with `unreachable_since` values for keys", + %{result: result, url1: url1, url2: url2} do + assert is_map(result) + assert %NaiveDateTime{} = result[url1] + assert is_nil(result[url2]) + end + + test "returns an empty map for empty list or list containing no hosts / url" do + assert %{} == Instances.filter_reachable([]) + assert %{} == Instances.filter_reachable([nil]) end end