Merge pull request #373 from basho/jdm-make-backends-shutdown-on-drop

Changed semantics of backend:drop - backend must close all handles.
basho · Jul 25, 2012 · 2a45808 · 2a45808
2 parents 25d69ad + 6af4c10
commit 2a45808
Show file tree

Hide file tree

Showing 3 changed files with 36 additions and 45 deletions.
diff --git a/src/riak_kv_bitcask_backend.erl b/src/riak_kv_bitcask_backend.erl
@@ -124,7 +124,12 @@ start(Partition, Config) ->
 %% @doc Stop the bitcask backend
 -spec stop(state()) -> ok.
 stop(#state{ref=Ref}) ->
-    bitcask:close(Ref).
+    case Ref of
+        undefined ->
+            ok;
+        _ ->
+            bitcask:close(Ref)
+    end.
 
 %% @doc Retrieve an object from the bitcask backend
 -spec get(riak_object:bucket(), riak_object:key(), state()) ->
@@ -304,8 +309,7 @@ fold_objects(FoldObjectsFun, Acc, Opts, #state{opts=BitcaskOpts,
 -spec drop(state()) -> {ok, state()} | {error, term(), state()}.
 drop(#state{ref=Ref,
             partition=Partition,
-            root=DataRoot,
-            opts=BitcaskOpts}=State) ->
+            root=DataRoot}=State) ->
     %% Close the bitcask reference
     bitcask:close(Ref),
 
@@ -321,32 +325,17 @@ drop(#state{ref=Ref,
     CleanupDir = check_for_cleanup_dir(DataRoot, auto),
     move_unused_dirs(CleanupDir, PartitionDirs),
 
+    %% Spawn a process to cleanup the old data files.
+    %% The use of spawn is intentional. We do not
+    %% care if this process dies since any lingering
+    %% files will be cleaned up on the next drop.
+    %% The worst case is that the files hang
+    %% around and take up some disk space.
+    spawn(drop_data_cleanup(PartitionStr, CleanupDir)),
+
     %% Make sure the data directory is now empty
     data_directory_cleanup(PartitionDir),
-
-    case make_data_dir(filename:join([DataRoot,
-                                      PartitionStr])) of
-        {ok, DataDir} ->
-            %% Spawn a process to cleanup the old data files.
-            %% The use of spawn is intentional. We do not
-            %% care if this process dies since any lingering
-            %% files will be cleaned up on the next drop.
-            %% The worst case is that the files hang
-            %% around and take up some disk space.
-            spawn(drop_data_cleanup(PartitionStr, CleanupDir)),
-
-            %% Now open the bitcask and return an updated state
-            %% so this backend can continue processing.
-            case bitcask:open(filename:join(DataRoot, DataDir), BitcaskOpts) of
-                Ref1 when is_reference(Ref1) ->
-                    {ok, State#state{data_dir=DataDir,
-                                     ref=Ref1}};
-                {error, Reason} ->
-                    {error, Reason, State#state{data_dir=DataDir}}
-            end;
-        {error, Reason1} ->
-            {error, Reason1, State}
-    end.
+    {ok, State#state{ref = undefined}}.
 
 %% @doc Returns true if this bitcasks backend contains any
 %% non-tombstone values; otherwise returns false.
@@ -737,7 +726,7 @@ drop_test() ->
     %% Stop the backend
     ok = stop(State1),
     os:cmd("rm -rf test/bitcask-backend/*"),
-    ?assertEqual(["42", "auto_cleanup"], lists:sort(DataDirs)),
+    ?assertEqual(["auto_cleanup"], lists:sort(DataDirs)),
     %% The drop cleanup happens in a separate process so
     %% there is no guarantee it has happened yet when
     %% this test runs.

diff --git a/src/riak_kv_eleveldb_backend.erl b/src/riak_kv_eleveldb_backend.erl
@@ -106,8 +106,13 @@ start(Partition, Config) ->
 
 %% @doc Stop the eleveldb backend
 -spec stop(state()) -> ok.
-stop(_State) ->
-    %% No-op; GC handles cleanup
+stop(State) ->
+    case State#state.ref of
+        undefined ->
+            ok;
+        _ ->
+            eleveldb:close(State#state.ref)
+    end,
     ok.
 
 %% @doc Retrieve an object from the eleveldb backend
@@ -280,12 +285,7 @@ drop(State0) ->
     eleveldb:close(State0#state.ref),
     case eleveldb:destroy(State0#state.data_root, []) of
         ok ->
-            case open_db(State0) of
-                {ok, State} ->
-                    {ok, State};
-                {error, Reason} ->
-                    {error, Reason, State0}
-            end;
+            {ok, State0#state{ref = undefined}};
         {error, Reason} ->
             {error, Reason, State0}
     end.

diff --git a/test/backend_eqc.erl b/test/backend_eqc.erl
@@ -249,12 +249,13 @@ init_backend(Backend, _Volatile, Config) ->
     S.
 
 drop(Backend, State) ->
-    case Backend:drop(State) of
-        {ok, NewState} ->
-            NewState;
-        {error, _, NewState} ->
-            NewState
-    end.
+    State1 = case Backend:drop(State) of
+                 {ok, NewState} ->
+                     NewState;
+                 {error, _, NewState} ->
+                     NewState
+             end,
+    Backend:stop(State1).
 
 delete(Bucket, Key, Backend, BackendState, Indexes) ->
     IndexSpecs = [{remove, Idx, SKey} || {B,Idx,SKey,K} <- Indexes,
@@ -349,9 +350,10 @@ next_state_data(_From, _To, S, _R, {call, _M, put, [Bucket, Key, IndexSpecs, Val
 next_state_data(_From, _To, S, _R, {call, _M, delete, [Bucket, Key|_]}) ->
     S#qcst{d = orddict:erase({Bucket, Key}, S#qcst.d),
            i = remove_indexes(Bucket, Key, S#qcst.i)};
-next_state_data(_From, _To, S, R, {call, ?MODULE, drop, _}) ->
+next_state_data(_From, _To, S, _R, {call, ?MODULE, drop, _}) ->
+
     S#qcst{d=orddict:new(),
-           s=R,
+           s=undefined,
            i=ordsets:new()};
 next_state_data(_From, _To, S, _R, _C) ->
     S.
@@ -373,7 +375,7 @@ running(#qcst{backend=Backend,
      {history, {call, Backend, fold_keys, [fold_keys_fun(), get_fold_buffer(), fold_keys_opts(), State]}},
      {history, {call, Backend, fold_objects, [fold_objects_fun(), get_fold_buffer(), g_opts(), State]}},
      {history, {call, Backend, is_empty, [State]}},
-     {history, {call, ?MODULE, drop, [Backend, State]}},
+     {stopped, {call, ?MODULE, drop, [Backend, State]}},
      {stopped, {call, Backend, stop, [State]}}
     ].