Merge pull request #19 from simplifi/fix_stuck_loading

Fix pay day loan getting stuck in :loading when we hit a timeout

lib/pay_day_loan/load_state.ex

@@ -215,6 +215,22 @@ defmodule PayDayLoan.LoadState do
     keys_in_state(ets_table_id, :reload, limit)
   end
 
+  @doc """
+  Return the list of keys in the `:loading` state
+  """
+  @spec loading_keys(atom | :ets.tid()) :: [PayDayLoan.key()]
+  def loading_keys(ets_table_id) do
+    keys_in_state(ets_table_id, :loading)
+  end
+
+  @doc """
+  Return the list of keys in the `:reload_loading` state
+  """
+  @spec reload_loading_keys(atom | :ets.tid()) :: [PayDayLoan.key()]
+  def reload_loading_keys(ets_table_id) do
+    keys_in_state(ets_table_id, :reload_loading)
+  end
+
   @doc """
   Returns all elements of the table
   """
@@ -228,6 +244,12 @@ defmodule PayDayLoan.LoadState do
     status
   end
 
+  defp keys_in_state(ets_table_id, state) do
+    ets_table_id
+    |> :ets.match({:"$1", state})
+    |> List.flatten()
+  end
+
   defp keys_in_state(ets_table_id, state, limit) do
     case :ets.match(ets_table_id, {:"$1", state}, limit) do
       :"$end_of_table" -> []

lib/pay_day_loan/load_worker.ex

@@ -67,7 +67,20 @@ defmodule PayDayLoan.LoadWorker do
     {:noreply, %{state | load_task_ref: nil}}
   end
 
-  def handle_info({:DOWN, _ref, :process, _pid, _reason}, state) do
+  def handle_info({:DOWN, _ref, :process, _pid, _reason}, %{pdl: pdl} = state) do
+    # The loader process died, so reset the status of any keys that were in the :loading
+    # or :request_loading states to unloaded.
+
+    LoadState.unload(
+      pdl.load_state_manager,
+      LoadState.loading_keys(pdl.load_state_manager)
+    )
+
+    LoadState.unload(
+      pdl.load_state_manager,
+      LoadState.reload_loading_keys(pdl.load_state_manager)
+    )
+
     {:noreply, %{state | load_task_ref: nil}}
   end
 

test/pay_day_loan/backends/no_retry_test.exs

@@ -0,0 +1,68 @@
+defmodule PayDayLoan.Backends.NoRetryTest do
+  use ExUnit.Case
+
+  alias PayDayLoan, as: PDL
+
+  import PayDayLoanTest.Support, only: [wait_for: 1]
+
+  alias PayDayLoanTest.Support.LoadHistory
+  alias PayDayLoan.LoadState
+
+  # our cache for testing
+  defmodule Cache do
+    # we override batch_size here so we can test overrides -
+    #   this is not necessary in general use
+    use(
+      PayDayLoan,
+      batch_size: 10,
+      load_num_tries: 1,
+      load_wait_msec: 50,
+      callback_module: PayDayLoan.Backends.NoRetryTest.Implementation
+    )
+  end
+
+  # Dummy loader behaviour implementation
+  defmodule Implementation do
+    use PayDayLoan.Support.TestImplementation
+
+    def on_new(_key, value) do
+      {:ok, value}
+    end
+
+    def on_remove(_key, _value) do
+    end
+
+    def on_replace(_old_value, _key, _value) do
+      {:ok, "replaced"}
+    end
+
+    def on_update(_old_value, _key, value) do
+      {:ok, value}
+    end
+  end
+
+  use PayDayLoan.Support.CommonTests, cache: Cache
+
+  test "requesting a key that causes a timeout crash in the load process" do
+    reloading_key = Implementation.key_that_reloads_slowly()
+
+    Patiently.wait_for!(fn ->
+      {result, _} = Cache.get(reloading_key)
+      result == :ok
+    end)
+
+    LoadState.reload_loading(Cache.pdl().load_state_manager, reloading_key)
+
+    key = Implementation.key_that_times_out_during_bulk_load()
+    assert {:error, :timed_out} == Cache.get(key)
+
+    # The keys should no longer exist in the load state - unload is supposed to remove them.
+    assert nil == PDL.peek_load_state(Cache.pdl(), key)
+    assert nil == PDL.peek_load_state(Cache.pdl(), reloading_key)
+
+    # The final check is that even after a previous failure, the keys are marked requested
+    # when we try to get them:
+    assert :requested == PDL.query_load_state(Cache.pdl(), key)
+    assert :requested == PDL.query_load_state(Cache.pdl(), reloading_key)
+  end
+end

test/support/test_implementation.ex

@@ -15,6 +15,7 @@ defmodule PayDayLoan.Support.TestImplementation do
       @key_that_returns_ignore_on_new "key that returns ignore on new"
       @key_that_returns_ignore_on_refresh "key that returns ignore on refresh"
       @key_that_is_removed_from_backend "key that is removed from backend"
+      @key_that_times_out_during_bulk_load "key that times out during bulk load"
 
       # we'll refuse to load this key
       def key_that_shall_not_be_loaded do
@@ -67,9 +68,20 @@ defmodule PayDayLoan.Support.TestImplementation do
         @key_that_is_removed_from_backend
       end
 
+      # this key causes a timeout crash in the loader itself
+      def key_that_times_out_during_bulk_load do
+        @key_that_times_out_during_bulk_load
+      end
+
       def bulk_load(keys) do
         LoadHistory.loaded(keys)
 
+        if Enum.any?(keys, fn k -> k == key_that_times_out_during_bulk_load() end) do
+          # simulate a typical pattern match error that happens when we get a timeout
+          # during bulk load and don't recover gracefully
+          exit(:timed_out)
+        end
+
         keys =
           keys
           |> Enum.filter(fn k ->