Ensure all autoretries dont happen within the genserver's process so that the genserver is available to answer other genservers

Author: mgwidmann

config/config.exs

@@ -47,6 +47,11 @@ config :slack_coder, SlackCoder.Guardian,
   serializer: SlackCoder.Guardian,
   secret_key: "nUzHV60AayAhzalp8iRhB5FokH7tcVv67ozi6PTjO0PZcUtyO4uoQRfcrVk5bc54"
 
+config :httpoison_retry,
+  wait: 15_000,
+  max_attempts: 5,
+  include_404s: true
+
 config :scrivener_html,
   routes_helper: SlackCoder.Router.Helpers
 

config/dev.exs

@@ -42,6 +42,11 @@ config :logger,
 # and calculating stacktraces is usually expensive.
 config :phoenix, :stacktrace_depth, 20
 
+# Make dev mode more likely to fail but with still a little bit of error protection
+config :httpoison_retry,
+  wait: 1_000,
+  max_attempts: 2
+
 database = if(System.get_env("DATA_DB_USER") == "nanobox", do: "gonano", else: "slack_coder_dev")
 # Configure your database
 config :slack_coder, SlackCoder.Repo,

lib/services/pr_service.ex

@@ -26,7 +26,14 @@ defmodule SlackCoder.Services.PRService do
     |> Repo.insert_or_update()
     |> case do
       {:ok, pr} ->
-        {:ok, pr |> check_failed() |> notifications() |> broadcast(Map.to_list(changeset.changes), changeset.data.id == nil)}
+        pr_worker = self()
+        pr = pr |> notifications() |> broadcast(Map.to_list(changeset.changes), changeset.data.id == nil)
+        # This could take some time with the autoretry, so don't hold up the PR worker or others tryig to
+        # contact the worker will time out since the PR worker can be sleeping
+        Task.Supervisor.start_child SlackCoder.TaskSupervisor, fn ->
+          GenServer.cast(pr_worker, {:save, pr |> check_failed()})
+        end
+        {:ok, pr}
       errored_changeset ->
         Logger.error "Unable to save PR: #{inspect errored_changeset}\n#{inspect changeset.data}"
         errored_changeset

lib/slack_coder/github/watchers/merge_conflict.ex

@@ -16,7 +16,7 @@ defmodule SlackCoder.Github.Watchers.MergeConflict do
     GenServer.start_link(__MODULE__, %{prs: []}, name: __MODULE__)
   end
 
-  @normal_wait_time 60 * 5 * 1_000
+  @normal_wait_time 60 * 1 * 1_000
   def init(state) do
     Process.send_after(self(), :check_conflicts, @normal_wait_time)
     {:ok, state}
@@ -95,28 +95,38 @@ defmodule SlackCoder.Github.Watchers.MergeConflict do
     {:noreply, state}
   end
   def handle_info(:check_conflicts, %{prs: prs} = state) do
-    remaining_prs =
-      case SlackCoder.Github.query(@mergeable_query, variable_params(prs)) do
-        {:ok, %{"data" => data}} when is_map(data) ->
-          response_prs = Map.values(data) |> Enum.map(&(&1["pullRequest"])) |> Enum.filter(&(&1))
+    # This could take some time with the autoretry, so don't hold up the genserver or others tryig to
+    # contact the genserver will time out since the genserver can be sleeping
+    s = self()
+    Task.Supervisor.start_child SlackCoder.TaskSupervisor, fn ->
+      remaining_prs =
+        case SlackCoder.Github.query(@mergeable_query, variable_params(prs)) do
+          {:ok, %{"data" => data}} when is_map(data) ->
+            response_prs = Map.values(data) |> Enum.map(&(&1["pullRequest"])) |> Enum.filter(&(&1))
 
-          Enum.reject(prs, fn(pr) ->
-            response = Enum.find(response_prs, &(&1["number"] == pr.number && &1["repository"]["name"] == pr.repo && &1["repository"]["owner"]["login"] == pr.owner))
-            if response && response["mergeable"] != @unknown do
-              Logger.debug [IO.ANSI.green, IO.ANSI.bright, "[MergeConflict] ", IO.ANSI.normal, IO.ANSI.default_color, "Status of PR-", to_string(pr.number), " ", response["mergeable"]]
-              pr
-              |> Github.find_watcher()
-              |> PullRequest.update_sync(%{"mergeable_state" => response["mergeable"] |> convert_mergeable()})
-            end
-          end)
-        %HTTPoison.Error{reason: :timeout} ->
-          # Ignore (Rate limiting)
-          prs
-        error ->
-          Logger.warn "Received unexpected response from Github: #{inspect error}"
-          prs
-      end
+            Enum.reject(prs, fn(pr) ->
+              response = Enum.find(response_prs, &(&1["number"] == pr.number && &1["repository"]["name"] == pr.repo && &1["repository"]["owner"]["login"] == pr.owner))
+              if response && response["mergeable"] != @unknown do
+                Logger.debug [IO.ANSI.green, IO.ANSI.bright, "[MergeConflict] ", IO.ANSI.normal, IO.ANSI.default_color, "Status of PR-", to_string(pr.number), " ", response["mergeable"]]
+                pr
+                |> Github.find_watcher()
+                |> PullRequest.update_sync(%{"mergeable_state" => response["mergeable"] |> convert_mergeable()})
+              end
+            end)
+          %HTTPoison.Error{reason: :timeout} ->
+            # Ignore (Rate limiting)
+            prs
+          error ->
+            Logger.warn "Received unexpected response from Github: #{inspect error}"
+            prs
+        end
+      send(s, {:check_conflict_complete, remaining_prs})
+    end
 
+    {:noreply, state}
+  end
+
+  def handle_info({:check_conflict_complete, remaining_prs}, state) do
     Process.send_after(self(), :check_conflicts, check_conflict_timeout(remaining_prs))
     {:noreply, Map.put(state, :prs, remaining_prs)}
   end

lib/slack_coder/github/watchers/pull_request.ex

@@ -59,6 +59,10 @@ defmodule SlackCoder.Github.Watchers.PullRequest do
     {:noreply, {pr |> PRService.check_failed(), callouts}}
   end
 
+  def handle_cast({:save, pr}, {_pr, callouts}) do
+    {:noreply, {pr, callouts}}
+  end
+
   def handle_cast({:build, sha, url, state}, {%PR{sha: sha} = pr, callouts}) do
     {:ok, pr} = pr |> PR.reg_changeset(%{build_status: state, build_url: url}) |> PRService.save
     {:noreply, {pr, callouts}}