diff --git a/lib/philomena/adverts.ex b/lib/philomena/adverts.ex index 523f7e19..06a1104e 100644 --- a/lib/philomena/adverts.ex +++ b/lib/philomena/adverts.ex @@ -8,6 +8,7 @@ defmodule Philomena.Adverts do alias Philomena.Adverts.Advert alias Philomena.Adverts.Restrictions + alias Philomena.Adverts.Server alias Philomena.Adverts.Uploader @doc """ @@ -64,6 +65,32 @@ defmodule Philomena.Adverts do Repo.one(query) end + @doc """ + Asynchronously records a new impression. + + ## Example + + iex> record_impression(%Advert{}) + :ok + + """ + def record_impression(%Advert{id: id}) do + Server.record_impression(id) + end + + @doc """ + Asynchronously records a new click. + + ## Example + + iex> record_click(%Advert{}) + :ok + + """ + def record_click(%Advert{id: id}) do + Server.record_click(id) + end + @doc """ Gets a single advert. diff --git a/lib/philomena_web/advert_updater.ex b/lib/philomena/adverts/recorder.ex similarity index 50% rename from lib/philomena_web/advert_updater.ex rename to lib/philomena/adverts/recorder.ex index 02846f9e..19e15cf5 100644 --- a/lib/philomena_web/advert_updater.ex +++ b/lib/philomena/adverts/recorder.ex @@ -1,33 +1,9 @@ -defmodule PhilomenaWeb.AdvertUpdater do +defmodule Philomena.Adverts.Recorder do alias Philomena.Adverts.Advert alias Philomena.Repo import Ecto.Query - def child_spec([]) do - %{ - id: PhilomenaWeb.AdvertUpdater, - start: {PhilomenaWeb.AdvertUpdater, :start_link, [[]]} - } - end - - def start_link([]) do - {:ok, spawn_link(&init/0)} - end - - def cast(type, advert_id) when type in [:impression, :click] do - pid = Process.whereis(:advert_updater) - if pid, do: send(pid, {type, advert_id}) - end - - defp init do - Process.register(self(), :advert_updater) - run() - end - - defp run do - # Read impression counts from mailbox - {impressions, clicks} = receive_all() - + def run(%{impressions: impressions, clicks: clicks}) do now = DateTime.utc_now() |> DateTime.truncate(:second) # Create insert statements for Ecto @@ -41,24 +17,7 @@ defmodule PhilomenaWeb.AdvertUpdater do Repo.insert_all(Advert, impressions, on_conflict: impressions_update, conflict_target: [:id]) Repo.insert_all(Advert, clicks, on_conflict: clicks_update, conflict_target: [:id]) - :timer.sleep(:timer.seconds(10)) - - run() - end - - defp receive_all(impressions \\ %{}, clicks \\ %{}) do - receive do - {:impression, advert_id} -> - impressions = Map.update(impressions, advert_id, 1, &(&1 + 1)) - receive_all(impressions, clicks) - - {:click, advert_id} -> - clicks = Map.update(clicks, advert_id, 1, &(&1 + 1)) - receive_all(impressions, clicks) - after - 0 -> - {impressions, clicks} - end + :ok end defp impressions_insert_all({advert_id, impressions}, now) do diff --git a/lib/philomena/adverts/server.ex b/lib/philomena/adverts/server.ex new file mode 100644 index 00000000..be759693 --- /dev/null +++ b/lib/philomena/adverts/server.ex @@ -0,0 +1,94 @@ +defmodule Philomena.Adverts.Server do + @moduledoc """ + Advert impression and click aggregator. + + Updating the impression count for adverts and clicks on every pageload is unnecessary + and slows down requests. This module collects the adverts and clicks and submits a batch + of updates to the database after every 10 seconds asynchronously, reducing the amount of + work to be done. + """ + + use GenServer + alias Philomena.Adverts.Recorder + + @type advert_id :: integer() + + @doc """ + Starts the GenServer. + + See `GenServer.start_link/2` for more information. + """ + def start_link(_) do + GenServer.start_link(__MODULE__, [], name: __MODULE__) + end + + @doc """ + Asynchronously records a new impression. + + ## Example + + iex> record_impression(advert.id) + :ok + + """ + @spec record_impression(advert_id()) :: :ok + def record_impression(advert_id) do + GenServer.cast(__MODULE__, {:impressions, advert_id}) + end + + @doc """ + Asynchronously records a new click. + + ## Example + + iex> record_click(advert.id) + :ok + + """ + @spec record_click(advert_id()) :: :ok + def record_click(advert_id) do + GenServer.cast(__MODULE__, {:clicks, advert_id}) + end + + # Used to force the GenServer to immediately sleep when no + # messages are available. + @timeout 0 + @sleep :timer.seconds(10) + + @impl true + @doc false + def init(_) do + {:ok, initial_state(), @timeout} + end + + @impl true + @doc false + def handle_cast({type, advert_id}, state) do + # Update the counter described by the message + state = update_in(state[type], &increment_counter(&1, advert_id)) + + # Return to GenServer event loop + {:noreply, state, @timeout} + end + + @impl true + @doc false + def handle_info(:timeout, state) do + # Process all updates from state now + Recorder.run(state) + + # Sleep for the specified delay + :timer.sleep(@sleep) + + # Return to GenServer event loop + {:noreply, initial_state(), @timeout} + end + + defp increment_counter(map, advert_id) do + Map.update(map, advert_id, 1, &(&1 + 1)) + end + + defp initial_state do + %{impressions: %{}, clicks: %{}} + end +end diff --git a/lib/philomena/application.ex b/lib/philomena/application.ex index f85de948..3ecfcc03 100644 --- a/lib/philomena/application.ex +++ b/lib/philomena/application.ex @@ -28,8 +28,10 @@ defmodule Philomena.Application do node_name: valid_node_name(node()) ]}, + # Advert update batching + Philomena.Adverts.Server, + # Start the endpoint when the application starts - PhilomenaWeb.AdvertUpdater, PhilomenaWeb.UserFingerprintUpdater, PhilomenaWeb.UserIpUpdater, PhilomenaWeb.Endpoint diff --git a/lib/philomena_web/controllers/advert_controller.ex b/lib/philomena_web/controllers/advert_controller.ex index 9387fbd3..73c47482 100644 --- a/lib/philomena_web/controllers/advert_controller.ex +++ b/lib/philomena_web/controllers/advert_controller.ex @@ -1,15 +1,15 @@ defmodule PhilomenaWeb.AdvertController do use PhilomenaWeb, :controller - alias PhilomenaWeb.AdvertUpdater alias Philomena.Adverts.Advert + alias Philomena.Adverts plug :load_resource, model: Advert def show(conn, _params) do advert = conn.assigns.advert - AdvertUpdater.cast(:click, advert.id) + Adverts.record_click(advert) redirect(conn, external: advert.link) end diff --git a/lib/philomena_web/plugs/advert_plug.ex b/lib/philomena_web/plugs/advert_plug.ex index 8ed4f543..797184a3 100644 --- a/lib/philomena_web/plugs/advert_plug.ex +++ b/lib/philomena_web/plugs/advert_plug.ex @@ -1,5 +1,4 @@ defmodule PhilomenaWeb.AdvertPlug do - alias PhilomenaWeb.AdvertUpdater alias Philomena.Adverts alias Plug.Conn @@ -33,7 +32,7 @@ defmodule PhilomenaWeb.AdvertPlug do defp record_impression(nil), do: nil defp record_impression(advert) do - AdvertUpdater.cast(:impression, advert.id) + Adverts.record_impression(advert) advert end