You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@couchdb.apache.org by da...@apache.org on 2019/07/21 19:43:37 UTC
[couchdb] 01/01: Make sure that fsync errors are raised
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.