From ee67c98e550310813cfdb9242e5fab2e566e1e2a Mon Sep 17 00:00:00 2001 From: Alexander Strizhakov Date: Sun, 6 Sep 2020 12:13:26 +0300 Subject: [PATCH] removing Stats worker from Oban cron jobs --- CHANGELOG.md | 8 ++ config/config.exs | 3 +- config/description.exs | 1 - lib/mix/pleroma.ex | 1 + lib/pleroma/application.ex | 1 + lib/pleroma/config/oban.ex | 30 ++++++++ lib/pleroma/stats.ex | 76 ++++++++++++++----- lib/pleroma/workers/cron/stats_worker.ex | 17 ----- ...ove_cron_stats_worker_from_oban_config.exs | 19 +++++ test/stats_test.exs | 21 ++--- 10 files changed, 127 insertions(+), 50 deletions(-) create mode 100644 lib/pleroma/config/oban.ex delete mode 100644 lib/pleroma/workers/cron/stats_worker.ex create mode 100644 priv/repo/migrations/20200906072147_remove_cron_stats_worker_from_oban_config.exs diff --git a/CHANGELOG.md b/CHANGELOG.md index 3e6353a4e..78aae6f07 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,9 +6,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). ## unreleased-patch - ??? ### Added + - Rich media failure tracking (along with `:failure_backoff` option) ### Fixed + - Possible OOM errors with the default HTTP adapter - Mastodon API: Search parameter `following` now correctly returns the followings rather than the followers - Mastodon API: Timelines hanging for (`number of posts with links * rich media timeout`) in the worst case. @@ -16,6 +18,12 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/). - Mastodon API: Cards being wrong for preview statuses due to cache key collision - Password resets no longer processed for deactivated accounts +## Unreleased + +### Removed + +- **Breaking:** Removed `Pleroma.Workers.Cron.StatsWorker` setting from Oban `:crontab`. + ## [2.1.0] - 2020-08-28 ### Changed diff --git a/config/config.exs b/config/config.exs index ed37b93c0..d631c3962 100644 --- a/config/config.exs +++ b/config/config.exs @@ -546,7 +546,6 @@ plugins: [Oban.Plugins.Pruner], crontab: [ {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker}, - {"0 * * * *", Pleroma.Workers.Cron.StatsWorker}, {"* * * * *", Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker}, {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker} @@ -672,7 +671,7 @@ # With no frontend configuration, the bundled files from the `static` directory will # be used. # -# config :pleroma, :frontends, +# config :pleroma, :frontends, # primary: %{"name" => "pleroma-fe", "ref" => "develop"}, # admin: %{"name" => "admin-fe", "ref" => "stable"}, # available: %{...} diff --git a/config/description.exs b/config/description.exs index 5e08ba109..18c133f02 100644 --- a/config/description.exs +++ b/config/description.exs @@ -2291,7 +2291,6 @@ description: "Settings for cron background jobs", suggestions: [ {"0 0 * * *", Pleroma.Workers.Cron.ClearOauthTokenWorker}, - {"0 * * * *", Pleroma.Workers.Cron.StatsWorker}, {"* * * * *", Pleroma.Workers.Cron.PurgeExpiredActivitiesWorker}, {"0 0 * * 0", Pleroma.Workers.Cron.DigestEmailsWorker}, {"0 0 * * *", Pleroma.Workers.Cron.NewUsersDigestWorker} diff --git a/lib/mix/pleroma.ex b/lib/mix/pleroma.ex index fe9b0d16c..49ba2aae4 100644 --- a/lib/mix/pleroma.ex +++ b/lib/mix/pleroma.ex @@ -18,6 +18,7 @@ defmodule Mix.Pleroma do @doc "Common functions to be reused in mix tasks" def start_pleroma do Pleroma.Config.Holder.save_default() + Pleroma.Config.Oban.warn() Application.put_env(:phoenix, :serve_endpoints, false, persistent: true) if Pleroma.Config.get(:env) != :test do diff --git a/lib/pleroma/application.ex b/lib/pleroma/application.ex index 33b1e3872..c39e24919 100644 --- a/lib/pleroma/application.ex +++ b/lib/pleroma/application.ex @@ -50,6 +50,7 @@ def start(_type, _args) do Pleroma.Telemetry.Logger.attach() Config.Holder.save_default() Pleroma.HTML.compile_scrubbers() + Pleroma.Config.Oban.warn() Config.DeprecationWarnings.warn() Pleroma.Plugs.HTTPSecurityPlug.warn_if_disabled() Pleroma.ApplicationRequirements.verify!() diff --git a/lib/pleroma/config/oban.ex b/lib/pleroma/config/oban.ex new file mode 100644 index 000000000..c2d56ebab --- /dev/null +++ b/lib/pleroma/config/oban.ex @@ -0,0 +1,30 @@ +defmodule Pleroma.Config.Oban do + require Logger + + def warn do + oban_config = Pleroma.Config.get(Oban) + + crontab = + [Pleroma.Workers.Cron.StatsWorker] + |> Enum.reduce(oban_config[:crontab], fn removed_worker, acc -> + with acc when is_list(acc) <- acc, + setting when is_tuple(setting) <- + Enum.find(acc, fn {_, worker} -> worker == removed_worker end) do + """ + !!!OBAN CONFIG WARNING!!! + You are using old workers in Oban crontab settings, which were removed. + Please, remove setting from crontab in your config file (prod.secret.exs): #{ + inspect(setting) + } + """ + |> Logger.warn() + + List.delete(acc, setting) + else + _ -> acc + end + end) + + Pleroma.Config.put(Oban, Keyword.put(oban_config, :crontab, crontab)) + end +end diff --git a/lib/pleroma/stats.ex b/lib/pleroma/stats.ex index 9a03f01db..e7f8d272c 100644 --- a/lib/pleroma/stats.ex +++ b/lib/pleroma/stats.ex @@ -3,12 +3,15 @@ # SPDX-License-Identifier: AGPL-3.0-only defmodule Pleroma.Stats do + use GenServer + import Ecto.Query + alias Pleroma.CounterCache alias Pleroma.Repo alias Pleroma.User - use GenServer + @interval :timer.seconds(60) def start_link(_) do GenServer.start_link( @@ -18,6 +21,11 @@ def start_link(_) do ) end + @impl true + def init(_args) do + {:ok, nil, {:continue, :calculate_stats}} + end + @doc "Performs update stats" def force_update do GenServer.call(__MODULE__, :force_update) @@ -29,7 +37,11 @@ def do_collect do end @doc "Returns stats data" - @spec get_stats() :: %{domain_count: integer(), status_count: integer(), user_count: integer()} + @spec get_stats() :: %{ + domain_count: non_neg_integer(), + status_count: non_neg_integer(), + user_count: non_neg_integer() + } def get_stats do %{stats: stats} = GenServer.call(__MODULE__, :get_state) @@ -44,25 +56,14 @@ def get_peers do peers end - def init(_args) do - {:ok, calculate_stat_data()} - end - - def handle_call(:force_update, _from, _state) do - new_stats = calculate_stat_data() - {:reply, new_stats, new_stats} - end - - def handle_call(:get_state, _from, state) do - {:reply, state, state} - end - - def handle_cast(:run_update, _state) do - new_stats = calculate_stat_data() - - {:noreply, new_stats} - end - + @spec calculate_stat_data() :: %{ + peers: list(), + stats: %{ + domain_count: non_neg_integer(), + status_count: non_neg_integer(), + user_count: non_neg_integer() + } + } def calculate_stat_data do peers = from( @@ -97,6 +98,7 @@ def calculate_stat_data do } end + @spec get_status_visibility_count(String.t() | nil) :: map() def get_status_visibility_count(instance \\ nil) do if is_nil(instance) do CounterCache.get_sum() @@ -104,4 +106,36 @@ def get_status_visibility_count(instance \\ nil) do CounterCache.get_by_instance(instance) end end + + @impl true + def handle_continue(:calculate_stats, _) do + stats = calculate_stat_data() + Process.send_after(self(), :run_update, @interval) + {:noreply, stats} + end + + @impl true + def handle_call(:force_update, _from, _state) do + new_stats = calculate_stat_data() + {:reply, new_stats, new_stats} + end + + @impl true + def handle_call(:get_state, _from, state) do + {:reply, state, state} + end + + @impl true + def handle_cast(:run_update, _state) do + new_stats = calculate_stat_data() + + {:noreply, new_stats} + end + + @impl true + def handle_info(:run_update, _) do + new_stats = calculate_stat_data() + Process.send_after(self(), :run_update, @interval) + {:noreply, new_stats} + end end diff --git a/lib/pleroma/workers/cron/stats_worker.ex b/lib/pleroma/workers/cron/stats_worker.ex deleted file mode 100644 index 6a79540bc..000000000 --- a/lib/pleroma/workers/cron/stats_worker.ex +++ /dev/null @@ -1,17 +0,0 @@ -# Pleroma: A lightweight social networking server -# Copyright © 2017-2020 Pleroma Authors -# SPDX-License-Identifier: AGPL-3.0-only - -defmodule Pleroma.Workers.Cron.StatsWorker do - @moduledoc """ - The worker to update peers statistics. - """ - - use Oban.Worker, queue: "background" - - @impl Oban.Worker - def perform(_job) do - Pleroma.Stats.do_collect() - :ok - end -end diff --git a/priv/repo/migrations/20200906072147_remove_cron_stats_worker_from_oban_config.exs b/priv/repo/migrations/20200906072147_remove_cron_stats_worker_from_oban_config.exs new file mode 100644 index 000000000..022f21dc7 --- /dev/null +++ b/priv/repo/migrations/20200906072147_remove_cron_stats_worker_from_oban_config.exs @@ -0,0 +1,19 @@ +defmodule Pleroma.Repo.Migrations.RemoveCronStatsWorkerFromObanConfig do + use Ecto.Migration + + def change do + with %Pleroma.ConfigDB{} = config <- + Pleroma.ConfigDB.get_by_params(%{group: :pleroma, key: Oban}), + crontab when is_list(crontab) <- config.value[:crontab], + index when is_integer(index) <- + Enum.find_index(crontab, fn {_, worker} -> + worker == Pleroma.Workers.Cron.StatsWorker + end) do + updated_value = Keyword.put(config.value, :crontab, List.delete_at(crontab, index)) + + config + |> Ecto.Changeset.change(value: updated_value) + |> Pleroma.Repo.update() + end + end +end diff --git a/test/stats_test.exs b/test/stats_test.exs index f09d8d31a..74bf785b0 100644 --- a/test/stats_test.exs +++ b/test/stats_test.exs @@ -4,7 +4,10 @@ defmodule Pleroma.StatsTest do use Pleroma.DataCase + import Pleroma.Factory + + alias Pleroma.Stats alias Pleroma.Web.CommonAPI describe "user count" do @@ -13,7 +16,7 @@ test "it ignores internal users" do _internal = insert(:user, local: true, nickname: nil) _internal = Pleroma.Web.ActivityPub.Relay.get_actor() - assert match?(%{stats: %{user_count: 1}}, Pleroma.Stats.calculate_stat_data()) + assert match?(%{stats: %{user_count: 1}}, Stats.calculate_stat_data()) end end @@ -47,23 +50,23 @@ test "on new status" do end) assert %{"direct" => 3, "private" => 4, "public" => 1, "unlisted" => 2} = - Pleroma.Stats.get_status_visibility_count() + Stats.get_status_visibility_count() end test "on status delete" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{visibility: "public", status: "hey"}) - assert %{"public" => 1} = Pleroma.Stats.get_status_visibility_count() + assert %{"public" => 1} = Stats.get_status_visibility_count() CommonAPI.delete(activity.id, user) - assert %{"public" => 0} = Pleroma.Stats.get_status_visibility_count() + assert %{"public" => 0} = Stats.get_status_visibility_count() end test "on status visibility update" do user = insert(:user) {:ok, activity} = CommonAPI.post(user, %{visibility: "public", status: "hey"}) - assert %{"public" => 1, "private" => 0} = Pleroma.Stats.get_status_visibility_count() + assert %{"public" => 1, "private" => 0} = Stats.get_status_visibility_count() {:ok, _} = CommonAPI.update_activity_scope(activity.id, %{visibility: "private"}) - assert %{"public" => 0, "private" => 1} = Pleroma.Stats.get_status_visibility_count() + assert %{"public" => 0, "private" => 1} = Stats.get_status_visibility_count() end test "doesn't count unrelated activities" do @@ -75,7 +78,7 @@ test "doesn't count unrelated activities" do CommonAPI.repeat(activity.id, other_user) assert %{"direct" => 0, "private" => 0, "public" => 1, "unlisted" => 0} = - Pleroma.Stats.get_status_visibility_count() + Stats.get_status_visibility_count() end end @@ -110,10 +113,10 @@ test "single instance" do end) assert %{"direct" => 10, "private" => 0, "public" => 1, "unlisted" => 5} = - Pleroma.Stats.get_status_visibility_count(local_instance) + Stats.get_status_visibility_count(local_instance) assert %{"direct" => 0, "private" => 20, "public" => 0, "unlisted" => 0} = - Pleroma.Stats.get_status_visibility_count(instance2) + Stats.get_status_visibility_count(instance2) end end end