Unverified Commit a16739a3 authored by Wang Yuan's avatar Wang Yuan Committed by GitHub
Browse files

Redis exit for fsync error when the AOF fsync policy is 'always' (#8347)

With AOF policy of fsync "always", redis should respect the contract with the user
that on acknowledged write data is already synced on disk.

Redis was already exiting for AOF write error,  but don't care about fsync failure.
So to guarantee data safe, redis should exit for fsync error too (when the AOF fsync
policy is 'always').
parent 4355145a
...@@ -451,10 +451,11 @@ void flushAppendOnlyFile(int force) { ...@@ -451,10 +451,11 @@ void flushAppendOnlyFile(int force) {
/* Handle the AOF write error. */ /* Handle the AOF write error. */
if (server.aof_fsync == AOF_FSYNC_ALWAYS) { if (server.aof_fsync == AOF_FSYNC_ALWAYS) {
/* We can't recover when the fsync policy is ALWAYS since the /* We can't recover when the fsync policy is ALWAYS since the reply
* reply for the client is already in the output buffers, and we * for the client is already in the output buffers (both writes and
* have the contract with the user that on acknowledged write data * reads), and the changes to the db can't be rolled back. Since we
* is synced on disk. */ * have a contract with the user that on acknowledged or observed
* writes are is synced on disk, we must exit. */
serverLog(LL_WARNING,"Can't recover from AOF write error when the AOF fsync policy is 'always'. Exiting..."); serverLog(LL_WARNING,"Can't recover from AOF write error when the AOF fsync policy is 'always'. Exiting...");
exit(1); exit(1);
} else { } else {
...@@ -502,7 +503,14 @@ try_fsync: ...@@ -502,7 +503,14 @@ try_fsync:
/* redis_fsync is defined as fdatasync() for Linux in order to avoid /* redis_fsync is defined as fdatasync() for Linux in order to avoid
* flushing metadata. */ * flushing metadata. */
latencyStartMonitor(latency); latencyStartMonitor(latency);
redis_fsync(server.aof_fd); /* Let's try to get this data on the disk */ /* Let's try to get this data on the disk. To guarantee data safe when
* the AOF fsync policy is 'always', we should exit if failed to fsync
* AOF (see comment next to the exit(1) after write error above). */
if (redis_fsync(server.aof_fd) == -1) {
serverLog(LL_WARNING,"Can't persist AOF for fsync error when the "
"AOF fsync policy is 'always': %s. Exiting...", strerror(errno));
exit(1);
}
latencyEndMonitor(latency); latencyEndMonitor(latency);
latencyAddSampleIfNeeded("aof-fsync-always",latency); latencyAddSampleIfNeeded("aof-fsync-always",latency);
server.aof_fsync_offset = server.aof_current_size; server.aof_fsync_offset = server.aof_current_size;
...@@ -1847,6 +1855,16 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) { ...@@ -1847,6 +1855,16 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
close(newfd); close(newfd);
goto cleanup; goto cleanup;
} }
if (server.aof_fsync == AOF_FSYNC_EVERYSEC) {
aof_background_fsync(newfd);
} else if (server.aof_fsync == AOF_FSYNC_ALWAYS) {
if (redis_fsync(newfd) == -1) {
serverLog(LL_WARNING,
"Error trying to fsync the parent diff to the rewritten AOF: %s", strerror(errno));
close(newfd);
goto cleanup;
}
}
latencyEndMonitor(latency); latencyEndMonitor(latency);
latencyAddSampleIfNeeded("aof-rewrite-diff-write",latency); latencyAddSampleIfNeeded("aof-rewrite-diff-write",latency);
...@@ -1916,10 +1934,6 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) { ...@@ -1916,10 +1934,6 @@ void backgroundRewriteDoneHandler(int exitcode, int bysignal) {
/* AOF enabled, replace the old fd with the new one. */ /* AOF enabled, replace the old fd with the new one. */
oldfd = server.aof_fd; oldfd = server.aof_fd;
server.aof_fd = newfd; server.aof_fd = newfd;
if (server.aof_fsync == AOF_FSYNC_ALWAYS)
redis_fsync(newfd);
else if (server.aof_fsync == AOF_FSYNC_EVERYSEC)
aof_background_fsync(newfd);
server.aof_selected_db = -1; /* Make sure SELECT is re-issued */ server.aof_selected_db = -1; /* Make sure SELECT is re-issued */
aofUpdateCurrentSize(); aofUpdateCurrentSize();
server.aof_rewrite_base_size = server.aof_current_size; server.aof_rewrite_base_size = server.aof_current_size;
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment