Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep the log fd, don't re-open logfile in every logs #906

Open
wants to merge 3 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 5 additions & 9 deletions src/config.c
Original file line number Diff line number Diff line change
Expand Up @@ -561,17 +561,12 @@ void loadServerConfigFromString(char *config) {
argv = NULL;
}

if (server.logfile[0] != '\0') {
FILE *logfp;

serverLogOpen();
if (server.log_fd == -1) {
/* Test if we are able to open the file. The server will not
* be able to abort just for this problem later... */
logfp = fopen(server.logfile, "a");
if (logfp == NULL) {
err = sdscatprintf(sdsempty(), "Can't open the log file: %s", strerror(errno));
goto loaderr;
}
fclose(logfp);
err = sdscatprintf(sdsempty(), "Can't open the log file: %s", strerror(errno));
goto loaderr;
}

/* Sanity checks. */
Expand Down Expand Up @@ -2713,6 +2708,7 @@ static int setConfigDirOption(standardConfig *config, sds *argv, int argc, const
*err = strerror(errno);
return 0;
}
if (server.logfile[0] != '\0') serverLogOpen();
return 1;
}

Expand Down
7 changes: 2 additions & 5 deletions src/debug.c
Original file line number Diff line number Diff line change
Expand Up @@ -1607,15 +1607,12 @@ void logRegisters(ucontext_t *uc) {
*
* Close it with closeDirectLogFiledes(). */
int openDirectLogFiledes(void) {
int log_to_stdout = server.logfile[0] == '\0';
int fd = log_to_stdout ? STDOUT_FILENO : open(server.logfile, O_APPEND | O_CREAT | O_WRONLY, 0644);
return fd;
return server.log_fd;
}

/* Used to close what closeDirectLogFiledes() returns. */
void closeDirectLogFiledes(int fd) {
int log_to_stdout = server.logfile[0] == '\0';
if (!log_to_stdout) close(fd);
UNUSED(fd);
}

#if defined(HAVE_BACKTRACE) && defined(__linux__)
Expand Down
54 changes: 38 additions & 16 deletions src/server.c
Original file line number Diff line number Diff line change
Expand Up @@ -108,26 +108,34 @@ const char *replstateToString(int replstate);
* function of the server may be called from other threads. */
void nolocks_localtime(struct tm *tmp, time_t t, time_t tz, int dst);

void serverLogOpen(void) {
if (server.log_fd != -1 && server.log_fd != STDOUT_FILENO) close(server.log_fd);

if (server.logfile[0] != '\0') {
server.log_fd = open(server.logfile, O_APPEND | O_CREAT | O_WRONLY, 0644);
} else {
server.log_fd = STDOUT_FILENO;
}
}

/* Low level logging. To use only for very big messages, otherwise
* serverLog() is to prefer. */
void serverLogRaw(int level, const char *msg) {
const int syslogLevelMap[] = {LOG_DEBUG, LOG_INFO, LOG_NOTICE, LOG_WARNING};
const char *c = ".-*#";
FILE *fp;
char buf[64];
int rawmode = (level & LL_RAW);
int log_to_stdout = server.logfile[0] == '\0';

level &= 0xff; /* clear flags */
if (level < server.verbosity) return;

fp = log_to_stdout ? stdout : fopen(server.logfile, "a");
if (!fp) return;
if (server.log_fd == -1) serverLogOpen();
if (server.log_fd == -1) return;

ssize_t nwritten;
if (rawmode) {
fprintf(fp, "%s", msg);
nwritten = write(server.log_fd, msg, strlen(msg));
} else {
int off;
struct timeval tv;
int role_char;
pid_t pid = getpid();
Expand All @@ -136,20 +144,28 @@ void serverLogRaw(int level, const char *msg) {
gettimeofday(&tv, NULL);
struct tm tm;
nolocks_localtime(&tm, tv.tv_sec, server.timezone, daylight_active);
off = strftime(buf, sizeof(buf), "%d %b %Y %H:%M:%S.", &tm);
snprintf(buf + off, sizeof(buf) - off, "%03d", (int)tv.tv_usec / 1000);

if (server.sentinel_mode) {
role_char = 'X'; /* Sentinel. */
} else if (pid != server.pid) {
role_char = 'C'; /* RDB / AOF writing child. */
} else {
role_char = (server.primary_host ? 'S' : 'M'); /* replica or Primary. */
}
fprintf(fp, "%d:%c %s %c %s\n", (int)getpid(), role_char, buf, c[level], msg);

size_t len = snprintf(buf, sizeof(buf), "%d:%c ", (int)pid, role_char);
len += strftime(buf + len, sizeof(buf) - len, "%d %b %Y %H:%M:%S.", &tm);
len += snprintf(buf + len, sizeof(buf) - len, "%03d %c ", (int)tv.tv_usec / 1000, c[level]);

char newline[] = "\n";

struct iovec iov[3] = {{.iov_base = (void *)buf, .iov_len = len},
{.iov_base = (void *)msg, .iov_len = strlen(msg)},
{.iov_base = (void *)newline, strlen(newline)}};
nwritten = writev(server.log_fd, iov, 3);
}
fflush(fp);
UNUSED(nwritten);

if (!log_to_stdout) fclose(fp);
if (server.syslog_enabled) syslog(syslogLevelMap[level], "%s", msg);
}

Expand All @@ -171,12 +187,14 @@ void _serverLog(int level, const char *fmt, ...) {
See serverLogFromHandler. */
void serverLogRawFromHandler(int level, const char *msg) {
int fd;
int log_to_stdout = server.logfile[0] == '\0';
char buf[64];

if ((level & 0xff) < server.verbosity || (log_to_stdout && server.daemonize)) return;
fd = log_to_stdout ? STDOUT_FILENO : open(server.logfile, O_APPEND | O_CREAT | O_WRONLY, 0644);
if (fd == -1) return;
if ((level & 0xff) < server.verbosity || (server.log_fd == STDOUT_FILENO && server.daemonize)) return;

if (server.log_fd == -1) serverLogOpen();
if (server.log_fd == -1) return;

fd = server.log_fd;
if (level & LL_RAW) {
if (write(fd, msg, strlen(msg)) == -1) goto err;
} else {
Expand All @@ -190,7 +208,7 @@ void serverLogRawFromHandler(int level, const char *msg) {
if (write(fd, "\n", 1) == -1) goto err;
}
err:
if (!log_to_stdout) close(fd);
return;
}

/* An async-signal-safe version of serverLog. if LL_RAW is not included in level flags,
Expand Down Expand Up @@ -1980,6 +1998,7 @@ void initServerConfig(void) {
server.runid[CONFIG_RUN_ID_SIZE] = '\0';
changeReplicationId();
clearReplicationId2();
server.log_fd = -1;
server.hz = CONFIG_DEFAULT_HZ; /* Initialize it ASAP, even if it may get
updated later after loading the config.
This value may be used before the server
Expand Down Expand Up @@ -6863,6 +6882,9 @@ int main(int argc, char **argv) {
}
if (server.sentinel_mode) sentinelCheckConfigFile();

/* Make sure that after the configuration is loaded and before printing the logs. */
if (server.logfile[0] == '\0') server.log_fd = STDOUT_FILENO;

/* Do system checks */
#ifdef __linux__
linuxMemoryWarnings();
Expand Down
2 changes: 2 additions & 0 deletions src/server.h
Original file line number Diff line number Diff line change
Expand Up @@ -1967,6 +1967,7 @@ struct valkeyServer {
int replication_allowed; /* Are we allowed to replicate? */
/* Logging */
char *logfile; /* Path of log file */
int log_fd; /* File descriptor of currently log file. */
int syslog_enabled; /* Is syslog enabled? */
char *syslog_ident; /* Syslog ident */
int syslog_facility; /* Syslog facility */
Expand Down Expand Up @@ -3283,6 +3284,7 @@ void serverLogFromHandler(int level, const char *fmt, ...) __attribute__((format
void serverLogFromHandler(int level, const char *fmt, ...);
void _serverLog(int level, const char *fmt, ...);
#endif
void serverLogOpen(void);
void serverLogRaw(int level, const char *msg);
void serverLogRawFromHandler(int level, const char *msg);
void usage(void);
Expand Down
Loading