[couchdb] branch fix-fsync-error-handling created (now 9ba601d)

[da...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

davisp pushed a change to branch fix-fsync-error-handling
in repository https://gitbox.apache.org/repos/asf/couchdb.git.


      at 9ba601d  Make sure that fsync errors are raised

This branch includes the following new commits:

     new 9ba601d  Make sure that fsync errors are raised

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

[couchdb] 01/01: Make sure that fsync errors are raised

[da...@apache.org]

This is an automated email from the ASF dual-hosted git repository.

davisp pushed a commit to branch fix-fsync-error-handling
in repository https://gitbox.apache.org/repos/asf/couchdb.git

commit 9ba601dda7927e9a6737e2b3d494bc5d560a6d2b
Author: Paul J. Davis <pa...@gmail.com>
AuthorDate: Sun Jul 21 14:40:52 2019 -0500

    Make sure that fsync errors are raised
    
    This changes `couch_file` to ensure that errors are raised when a call
    to `fsync` fails. It will also stop the couch_file process to ensure
    that anything handling a failed `fsync` won't attempt to retry the
    operation and experience issues discovered by Postgres [1].
    
    [1] http://danluu.com/fsyncgate/
---
 src/couch/src/couch_file.erl        | 25 ++++++++++++++++++++++---
 src/couch/test/couch_file_tests.erl | 33 +++++++++++++++++++++++++++++++++
 2 files changed, 55 insertions(+), 3 deletions(-)

diff --git a/src/couch/src/couch_file.erl b/src/couch/src/couch_file.erl
index d6e4066..ac8565f 100644
--- a/src/couch/src/couch_file.erl
+++ b/src/couch/src/couch_file.erl
@@ -215,12 +215,26 @@ truncate(Fd, Pos) ->
 sync(Filepath) when is_list(Filepath) ->
     case file:open(Filepath, [append, raw]) of
         {ok, Fd} ->
-            try ok = file:sync(Fd) after ok = file:close(Fd) end;
+            try
+                case file:sync(Fd) of
+                    ok ->
+                        ok;
+                    Else ->
+                        erlang:error({fsync_error, Else})
+                end
+            after
+                ok = file:close(Fd)
+            end;
         {error, Error} ->
             erlang:error(Error)
     end;
 sync(Fd) ->
-    gen_server:call(Fd, sync, infinity).
+    case gen_server:call(Fd, sync, infinity) of
+        ok ->
+            ok;
+        Else ->
+            erlang:error({fsync_error, Else})
+    end.
 
 %%----------------------------------------------------------------------
 %% Purpose: Close the file.
@@ -462,7 +476,12 @@ handle_call({set_db_pid, Pid}, _From, #file{db_monitor=OldRef}=File) ->
     {reply, ok, File#file{db_monitor=Ref}};
 
 handle_call(sync, _From, #file{fd=Fd}=File) ->
-    {reply, file:sync(Fd), File};
+    case file:sync(Fd) of
+        ok ->
+            {reply, ok, File};
+        Error ->
+            {stop, Error, Error, File}
+    end;
 
 handle_call({truncate, Pos}, _From, #file{fd=Fd}=File) ->
     {ok, Pos} = file:position(Fd, Pos),
diff --git a/src/couch/test/couch_file_tests.erl b/src/couch/test/couch_file_tests.erl
index 34c1a16..6e393f2 100644
--- a/src/couch/test/couch_file_tests.erl
+++ b/src/couch/test/couch_file_tests.erl
@@ -498,3 +498,36 @@ make_delete_dir_test_case({RootDir, ViewDir}, DeleteAfterRename) ->
 remove_dir(Dir) ->
     [file:delete(File) || File <- filelib:wildcard(filename:join([Dir, "*"]))],
     file:del_dir(Dir).
+
+
+fsync_error_test_() ->
+    {
+        "Test fsync raises errors",
+        {
+            setup,
+            fun() ->
+                test_util:start(?MODULE, [ioq])
+            end,
+            fun(Ctx) ->
+                test_util:stop(Ctx)
+            end,
+            [
+                fun fsync_raises_errors/0
+            ]
+        }
+    }.
+
+
+fsync_raises_errors() ->
+    Fd = spawn(fun() -> fake_fsync_fd() end),
+    ?assertError({fsync_error, eio}, couch_file:sync(Fd)).
+
+
+fake_fsync_fd() ->
+    % Mocking gen_server did not go very
+    % well so faking the couch_file pid
+    % will have to do.
+    receive
+        {'$gen_call', From, sync} ->
+            gen:reply(From, eio)
+    end.