Unverified Commit cc284813 authored by Tian's avatar Tian Committed by GitHub
Browse files

Make cluster config file saving atomic and fsync acl (#10924)



As an outstanding part mentioned in #10737, we could just make the cluster config file and
ACL file saving done with a more safe and atomic pattern (write to temp file, fsync, rename, fsync dir).

The cluster config file uses an in-place overwrite and truncation (which was also used by the
main config file before #7824).
The ACL file is using the temp file and rename approach, but was missing an fsync.
Co-authored-by: default avatar朱天 <zhutian03@meituan.com>
parent 56828bab
...@@ -2264,7 +2264,7 @@ int ACLSaveToFile(const char *filename) { ...@@ -2264,7 +2264,7 @@ int ACLSaveToFile(const char *filename) {
/* Create a temp file with the new content. */ /* Create a temp file with the new content. */
tmpfilename = sdsnew(filename); tmpfilename = sdsnew(filename);
tmpfilename = sdscatfmt(tmpfilename,".tmp-%i-%I", tmpfilename = sdscatfmt(tmpfilename,".tmp-%i-%I",
(int)getpid(),(int)mstime()); (int) getpid(),mstime());
if ((fd = open(tmpfilename,O_WRONLY|O_CREAT,0644)) == -1) { if ((fd = open(tmpfilename,O_WRONLY|O_CREAT,0644)) == -1) {
serverLog(LL_WARNING,"Opening temp ACL file for ACL SAVE: %s", serverLog(LL_WARNING,"Opening temp ACL file for ACL SAVE: %s",
strerror(errno)); strerror(errno));
...@@ -2272,8 +2272,19 @@ int ACLSaveToFile(const char *filename) { ...@@ -2272,8 +2272,19 @@ int ACLSaveToFile(const char *filename) {
} }
/* Write it. */ /* Write it. */
if (write(fd,acl,sdslen(acl)) != (ssize_t)sdslen(acl)) { size_t offset = 0;
serverLog(LL_WARNING,"Writing ACL file for ACL SAVE: %s", while (offset < sdslen(acl)) {
ssize_t written_bytes = write(fd,acl + offset,sdslen(acl) - offset);
if (written_bytes <= 0) {
if (errno == EINTR) continue;
serverLog(LL_WARNING,"Writing ACL file for ACL SAVE: %s",
strerror(errno));
goto cleanup;
}
offset += written_bytes;
}
if (redis_fsync(fd) == -1) {
serverLog(LL_WARNING,"Syncing ACL file for ACL SAVE: %s",
strerror(errno)); strerror(errno));
goto cleanup; goto cleanup;
} }
...@@ -2285,6 +2296,11 @@ int ACLSaveToFile(const char *filename) { ...@@ -2285,6 +2296,11 @@ int ACLSaveToFile(const char *filename) {
strerror(errno)); strerror(errno));
goto cleanup; goto cleanup;
} }
if (fsyncFileDir(filename) == -1) {
serverLog(LL_WARNING,"Syncing ACL directory for ACL SAVE: %s",
strerror(errno));
goto cleanup;
}
sdsfree(tmpfilename); tmpfilename = NULL; sdsfree(tmpfilename); tmpfilename = NULL;
retval = C_OK; /* If we reached this point, everything is fine. */ retval = C_OK; /* If we reached this point, everything is fine. */
......
...@@ -406,10 +406,11 @@ fmterr: ...@@ -406,10 +406,11 @@ fmterr:
* bigger we pad our payload with newlines that are anyway ignored and truncate * bigger we pad our payload with newlines that are anyway ignored and truncate
* the file afterward. */ * the file afterward. */
int clusterSaveConfig(int do_fsync) { int clusterSaveConfig(int do_fsync) {
sds ci; sds ci,tmpfilename;
size_t content_size; size_t content_size,offset = 0;
struct stat sb; ssize_t written_bytes;
int fd; int fd = -1;
int retval = C_ERR;
server.cluster->todo_before_sleep &= ~CLUSTER_TODO_SAVE_CONFIG; server.cluster->todo_before_sleep &= ~CLUSTER_TODO_SAVE_CONFIG;
...@@ -421,36 +422,52 @@ int clusterSaveConfig(int do_fsync) { ...@@ -421,36 +422,52 @@ int clusterSaveConfig(int do_fsync) {
(unsigned long long) server.cluster->lastVoteEpoch); (unsigned long long) server.cluster->lastVoteEpoch);
content_size = sdslen(ci); content_size = sdslen(ci);
if ((fd = open(server.cluster_configfile,O_WRONLY|O_CREAT,0644)) /* Create a temp file with the new content. */
== -1) goto err; tmpfilename = sdscatfmt(sdsempty(),"%s.tmp-%i-%I",
server.cluster_configfile,(int) getpid(),mstime());
if (redis_fstat(fd,&sb) == -1) goto err; if ((fd = open(tmpfilename,O_WRONLY|O_CREAT,0644)) == -1) {
serverLog(LL_WARNING,"Could not open temp cluster config file: %s",strerror(errno));
goto cleanup;
}
/* Pad the new payload if the existing file length is greater. */ while (offset < content_size) {
if (sb.st_size > (off_t)content_size) { written_bytes = write(fd,ci + offset,content_size - offset);
ci = sdsgrowzero(ci,sb.st_size); if (written_bytes <= 0) {
memset(ci+content_size,'\n',sb.st_size-content_size); if (errno == EINTR) continue;
serverLog(LL_WARNING,"Failed after writing (%zd) bytes to tmp cluster config file: %s",
offset,strerror(errno));
goto cleanup;
}
offset += written_bytes;
} }
if (write(fd,ci,sdslen(ci)) != (ssize_t)sdslen(ci)) goto err;
if (do_fsync) { if (do_fsync) {
server.cluster->todo_before_sleep &= ~CLUSTER_TODO_FSYNC_CONFIG; server.cluster->todo_before_sleep &= ~CLUSTER_TODO_FSYNC_CONFIG;
if (fsync(fd) == -1) goto err; if (redis_fsync(fd) == -1) {
serverLog(LL_WARNING,"Could not sync tmp cluster config file: %s",strerror(errno));
goto cleanup;
}
} }
/* Truncate the file if needed to remove the final \n padding that if (rename(tmpfilename, server.cluster_configfile) == -1) {
* is just garbage. */ serverLog(LL_WARNING,"Could not rename tmp cluster config file: %s",strerror(errno));
if (content_size != sdslen(ci) && ftruncate(fd,content_size) == -1) { goto cleanup;
/* ftruncate() failing is not a critical error. */
} }
close(fd);
sdsfree(ci);
return 0;
err: if (do_fsync) {
if (fsyncFileDir(server.cluster_configfile) == -1) {
serverLog(LL_WARNING,"Could not sync cluster config file dir: %s",strerror(errno));
goto cleanup;
}
}
retval = C_OK; /* If we reached this point, everything is fine. */
cleanup:
if (fd != -1) close(fd); if (fd != -1) close(fd);
if (retval) unlink(tmpfilename);
sdsfree(tmpfilename);
sdsfree(ci); sdsfree(ci);
return -1; return retval;
} }
void clusterSaveConfigOrDie(int do_fsync) { void clusterSaveConfigOrDie(int do_fsync) {
......
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