Minor refactoring

read_recovery_terms hides unused keys, better name for clean shutdown
checking and comment explaining why recovery_indexes is a gen_server.

3 files changed, 20 insertions, 16 deletions

diff --git a/src/rabbit_queue_index.erl b/src/rabbit_queue_index.erl
index 44ea8621e9..4ebcdfafc5 100644
--- a/src/rabbit_queue_index.erl
+++ b/src/rabbit_queue_index.erl
@@ -245,15 +245,15 @@ init(Name, OnSyncFun) ->
 shutdown_terms(Name) ->
     #qistate { dir = Dir } = blank_state(Name),
     case rabbit_recovery_indexes:read_recovery_terms(Dir) of
-        {error, _}        -> [];
-        {ok, {_, Terms1}} -> Terms1
+        {error, _}   -> [];
+        {ok, Terms1} -> Terms1
     end.
 
 recover(Name, Terms, MsgStoreRecovered, ContainsCheckFun, OnSyncFun) ->
     State = #qistate { dir = Dir } = blank_state(Name),
     State1 = State #qistate { on_sync = OnSyncFun },
     CleanShutdown =
-        rabbit_recovery_indexes:had_clean_shutdown(Dir),
+        rabbit_recovery_indexes:check_clean_shutdown(Dir),
     case CleanShutdown andalso MsgStoreRecovered of
         true  -> RecoveredCounts = proplists:get_value(segments, Terms, []),
                  init_clean(RecoveredCounts, State1);
diff --git a/src/rabbit_recovery_indexes.erl b/src/rabbit_recovery_indexes.erl
index dd7f06962d..460aaa1c7c 100644
--- a/src/rabbit_recovery_indexes.erl
+++ b/src/rabbit_recovery_indexes.erl
@@ -14,6 +14,9 @@
 %% Copyright (c) 2007-2013 GoPivotal, Inc.  All rights reserved.
 %%
 
+%% We use a gen_server simply so that during the terminate/2 call
+%% (i.e., during shutdown), we can sync/flush the dets table to disk.
+
 -module(rabbit_recovery_indexes).
 
 -behaviour(gen_server).
@@ -21,7 +24,7 @@
 -export([recover/0,
          start_link/0,
          store_recovery_terms/2,
-         had_clean_shutdown/1,
+         check_clean_shutdown/1,
          read_recovery_terms/1,
          remove_recovery_terms/1,
          flush/0]).
@@ -40,7 +43,7 @@
 -spec(store_recovery_terms(
         Name  :: rabbit_misc:resource_name(),
         Terms :: term()) -> rabbit_types:ok_or_error(term())).
--spec(had_clean_shutdown(
+-spec(check_clean_shutdown(
         rabbit_misc:resource_name()) ->
              boolean() | rabbit_types:error(term())).
 -spec(read_recovery_terms(
@@ -54,10 +57,10 @@
 -define(CLEAN_FILENAME, "clean.dot").
 
 recover() ->
-    {ok, _Child} = supervisor:start_child(rabbit_sup,
-                                          {?SERVER, {?MODULE, start_link, []},
-                                           permanent, ?MAX_WAIT, worker,
-                                           [?SERVER]}),
+    supervisor:start_child(rabbit_sup,
+                           {?SERVER, {?MODULE, start_link, []},
+                            permanent, ?MAX_WAIT, worker,
+                            [?SERVER]}),
     ok.
 
 start_link() ->
@@ -66,23 +69,24 @@ start_link() ->
 store_recovery_terms(Name, Terms) ->
     dets:insert(?MODULE, {Name, Terms}).
 
-had_clean_shutdown(Name) ->
+check_clean_shutdown(Name) ->
     ok == remove_recovery_terms(Name).
 
 read_recovery_terms(Name) ->
     case dets:lookup(?MODULE, Name) of
-        [Terms] -> {ok, Terms};
-        _       -> {error, not_found}
+        [{_, Terms}] -> {ok, Terms};
+        _            -> {error, not_found}
     end.
 
 remove_recovery_terms(Name) ->
     case dets:member(?MODULE, Name) of
-        true  -> dets:delete(?MODULE, Name);
-        false -> {error, not_found}
+        true -> dets:delete(?MODULE, Name);
+        _    -> {error, not_found}
     end.
 
 flush() ->
-    ok = dets:sync(?MODULE).
+    dets:sync(?MODULE),
+    ok.
 
 init(_) ->
     File = filename:join([rabbit_mnesia:dir(), "queues", ?CLEAN_FILENAME]),
diff --git a/src/rabbit_variable_queue.erl b/src/rabbit_variable_queue.erl
index 43e8a3b875..ac2b9f52d0 100644
--- a/src/rabbit_variable_queue.erl
+++ b/src/rabbit_variable_queue.erl
@@ -391,7 +391,7 @@
 start(DurableQueues) ->
     {AllTerms, StartFunState} = rabbit_queue_index:recover(DurableQueues),
     start_msg_store(
-      [Ref || {_, Terms} <- AllTerms,
+      [Ref || Terms <- AllTerms,
               begin
                   Ref = proplists:get_value(persistent_ref, Terms),
                   Ref =/= undefined