From 6b297cd64627a383ccb8d2d5a2ad736118a014ed Mon Sep 17 00:00:00 2001 From: Oran Agra Date: Sun, 24 Oct 2021 16:52:44 +0300 Subject: [PATCH] Improve errno reporting on fork and fopen rdbLoad failures (#9649) I moved a bunch of stats in redisFork to be executed only on successful fork, since they seem wrong to be done when it failed. I guess when fork fails it does that immediately, no latency spike. --- src/rdb.c | 2 +- src/replication.c | 2 +- src/scripting.c | 2 +- src/server.c | 11 +++++++---- 4 files changed, 10 insertions(+), 7 deletions(-) diff --git a/src/rdb.c b/src/rdb.c index cb4351ea6..afdf57daf 100644 --- a/src/rdb.c +++ b/src/rdb.c @@ -3110,7 +3110,6 @@ int rdbSaveToSlavesSockets(rdbSaveInfo *rsi) { exitFromChild((retval == C_OK) ? 0 : 1); } else { /* Parent */ - close(safe_to_exit_pipe); if (childpid == -1) { serverLog(LL_WARNING,"Can't save in background: fork: %s", strerror(errno)); @@ -3141,6 +3140,7 @@ int rdbSaveToSlavesSockets(rdbSaveInfo *rsi) { serverPanic("Unrecoverable error creating server.rdb_pipe_read file event."); } } + close(safe_to_exit_pipe); return (childpid == -1) ? C_ERR : C_OK; } return C_OK; /* Unreached. */ diff --git a/src/replication.c b/src/replication.c index eb574d0ea..1c8836c02 100644 --- a/src/replication.c +++ b/src/replication.c @@ -1859,7 +1859,7 @@ void readSyncBulkPayload(connection *conn) { if (rdbLoad(server.rdb_filename,&rsi,RDBFLAGS_REPLICATION) != C_OK) { serverLog(LL_WARNING, "Failed trying to load the MASTER synchronization " - "DB from disk"); + "DB from disk: %s", strerror(errno)); cancelReplicationHandshake(1); if (server.rdb_del_sync_files && allPersistenceDisabled()) { serverLog(LL_NOTICE,"Removing the RDB file obtained from " diff --git a/src/scripting.c b/src/scripting.c index 374ebaf91..85504bdb0 100644 --- a/src/scripting.c +++ b/src/scripting.c @@ -2164,7 +2164,7 @@ int ldbStartSession(client *c) { if (ldb.forked) { pid_t cp = redisFork(CHILD_TYPE_LDB); if (cp == -1) { - addReplyError(c,"Fork() failed: can't run EVAL in debugging mode."); + addReplyErrorFormat(c,"Fork() failed: can't run EVAL in debugging mode: %s", strerror(errno)); return 0; } else if (cp == 0) { /* Child. Let's ignore important signals handled by the parent. */ diff --git a/src/server.c b/src/server.c index a064b8032..ee1b689a6 100644 --- a/src/server.c +++ b/src/server.c @@ -7405,14 +7405,17 @@ int redisFork(int purpose) { closeChildUnusedResourceAfterFork(); } else { /* Parent */ + if (childpid == -1) { + int fork_errno = errno; + if (isMutuallyExclusiveChildType(purpose)) closeChildInfoPipe(); + errno = fork_errno; + return -1; + } + server.stat_total_forks++; server.stat_fork_time = ustime()-start; server.stat_fork_rate = (double) zmalloc_used_memory() * 1000000 / server.stat_fork_time / (1024*1024*1024); /* GB per second. */ latencyAddSampleIfNeeded("fork",server.stat_fork_time/1000); - if (childpid == -1) { - if (isMutuallyExclusiveChildType(purpose)) closeChildInfoPipe(); - return -1; - } /* The child_pid and child_type are only for mutual exclusive children. * other child types should handle and store their pid's in dedicated variables.