PGPRO-584: Fix logging options

Replace --log-level by --log-level-console and --log-level-file

Author: za-arthur

src/configure.c

@@ -9,7 +9,8 @@
 
 #include "pg_probackup.h"
 
-static void opt_log_level(pgut_option *opt, const char *arg);
+static void opt_log_level_console(pgut_option *opt, const char *arg);
+static void opt_log_level_file(pgut_option *opt, const char *arg);
 static void opt_compress_alg(pgut_option *opt, const char *arg);
 
 static pgBackupConfig *cur_config = NULL;
@@ -41,10 +42,10 @@ do_configure(bool show_only)
 	if (replica_timeout != 300)		/* 300 is default value */
 		config->replica_timeout = replica_timeout;
 
-	if (log_to_file != LOGGER_NONE)
-		config->log_to_file = LOG_TO_FILE;
-	if (log_level != LOGGER_NONE)
-		config->log_level = LOG_LEVEL;
+	if (log_level_console != LOG_NONE)
+		config->log_level_console = LOG_LEVEL_CONSOLE;
+	if (log_level_file != LOG_NONE)
+		config->log_level_file = LOG_LEVEL_FILE;
 	if (log_filename)
 		config->log_filename = log_filename;
 	if (error_log_filename)
@@ -90,8 +91,8 @@ pgBackupConfigInit(pgBackupConfig *config)
 	config->master_user = NULL;
 	config->replica_timeout = INT_MIN;	/* INT_MIN means "undefined" */
 
-	config->log_to_file = INT_MIN;		/* INT_MIN means "undefined" */
-	config->log_level = INT_MIN;		/* INT_MIN means "undefined" */
+	config->log_level_console = INT_MIN;	/* INT_MIN means "undefined" */
+	config->log_level_file = INT_MIN;		/* INT_MIN means "undefined" */
 	config->log_filename = NULL;
 	config->error_log_filename = NULL;
 	config->log_directory = NULL;
@@ -135,10 +136,10 @@ writeBackupCatalogConfig(FILE *out, pgBackupConfig *config)
 		fprintf(out, "replica_timeout = %d\n", config->replica_timeout);
 
 	fprintf(out, "#Logging parameters:\n");
-	if (config->log_to_file != INT_MIN)
-		fprintf(out, "log = %d\n", config->log_to_file);
-	if (config->log_level != INT_MIN)
-		fprintf(out, "log-level = %s\n", deparse_log_level(config->log_level));
+	if (config->log_level_console != INT_MIN)
+		fprintf(out, "log-level-console = %s\n", deparse_log_level(config->log_level_console));
+	if (config->log_level_file != INT_MIN)
+		fprintf(out, "log-level-file = %s\n", deparse_log_level(config->log_level_file));
 	if (config->log_filename)
 		fprintf(out, "log-filename = %s\n", config->log_filename);
 	if (config->error_log_filename)
@@ -198,8 +199,8 @@ readBackupCatalogConfigFile(void)
 		{ 'f', 0, "compress-algorithm",		opt_compress_alg,				SOURCE_CMDLINE },
 		{ 'u', 0, "compress-level",			&(config->compress_level),		SOURCE_CMDLINE },
 		/* logging options */
-		{ 'b', 0, "log",					&(config->log_to_file),			SOURCE_CMDLINE },
-		{ 'f', 0, "log-level",				opt_log_level,					SOURCE_CMDLINE },
+		{ 'f', 0, "log-level-console",		opt_log_level_console,			SOURCE_CMDLINE },
+		{ 'f', 0, "log-level-file",			opt_log_level_file,				SOURCE_CMDLINE },
 		{ 's', 0, "log-filename",			&(config->log_filename),		SOURCE_CMDLINE },
 		{ 's', 0, "error-log-filename",		&(config->error_log_filename),	SOURCE_CMDLINE },
 		{ 's', 0, "log-directory",			&(config->log_directory),		SOURCE_CMDLINE },
@@ -234,9 +235,15 @@ readBackupCatalogConfigFile(void)
 }
 
 static void
-opt_log_level(pgut_option *opt, const char *arg)
+opt_log_level_console(pgut_option *opt, const char *arg)
 {
-	cur_config->log_level = parse_log_level(arg);
+	cur_config->log_level_console = parse_log_level(arg);
+}
+
+static void
+opt_log_level_file(pgut_option *opt, const char *arg)
+{
+	cur_config->log_level_file = parse_log_level(arg);
 }
 
 static void

src/help.c

@@ -66,7 +66,8 @@ help_pg_probackup(void)
 	printf(_("\n  %s init -B backup-path [-l]\n"), PROGRAM_NAME);
 
 	printf(_("\n  %s set-config -B backup-dir --instance=instance_name\n"), PROGRAM_NAME);
-	printf(_("                 [--log-level=log-level]\n"));
+	printf(_("                 [--log-level-console=log-level-console]\n"));
+	printf(_("                 [--log-level-file=log-level-file]\n"));
 	printf(_("                 [--log-filename=log-filename]\n"));
 	printf(_("                 [--error-log-filename=error-log-filename]\n"));
 	printf(_("                 [--log-directory=log-directory]\n"));
@@ -132,7 +133,6 @@ help_init(void)
 {
 	printf(_("%s init -B backup-path -D pgdata-dir [-l]\n\n"), PROGRAM_NAME);
 	printf(_("  -B, --backup-path=backup-path    location of the backup storage area\n"));
-	printf(_("  -l, --log                        store messages in a log file\n"));
 }
 
 static void
@@ -153,7 +153,6 @@ help_backup(void)
 	printf(_("  -b, --backup-mode=backup-mode    backup mode=FULL|PAGE|PTRACK\n"));
 	printf(_("      --instance=instance_name     name of the instance\n"));
 	printf(_("  -C, --smooth-checkpoint          do smooth checkpoint before backup\n"));
-	printf(_("  -l, --log                        store messages in a log file\n"));
 	printf(_("      --stream                     stream the transaction log and include it in the backup\n"));
 	printf(_("      --archive-timeout            wait timeout for WAL segment archiving\n"));
 	printf(_("  -S, --slot=SLOTNAME              replication slot to use\n"));
@@ -195,7 +194,6 @@ help_restore(void)
 	printf(_("      --instance=instance_name     name of the instance\n"));
 
 	printf(_("  -D, --pgdata=pgdata-dir          location of the database storage area\n"));
-	printf(_("  -l, --log                        store messages in a log file\n"));
 	printf(_("  -i, --backup-id=backup-id        backup to restore\n"));
 
 	printf(_("      --progress                   show progress\n"));
@@ -219,7 +217,6 @@ help_validate(void)
 	printf(_("      --instance=instance_name     name of the instance\n"));
 	printf(_("  -i, --backup-id=backup-id        backup to validate\n"));
 
-	printf(_("  -l, --log                        store messages in a log file\n"));
 	printf(_("      --progress                   show progress\n"));
 	printf(_("      --time=time                  time stamp up to which recovery will proceed\n"));
 	printf(_("      --xid=xid                    transaction ID up to which recovery will proceed\n"));
@@ -250,14 +247,14 @@ help_delete(void)
 	printf(_("  -i, --backup-id=backup-id        backup to delete\n"));
 	printf(_("      --expired                    delete backups expired according to current\n"));
 	printf(_("                                   retention policy\n"));
-	printf(_("  -l, --log                        store messages in a log file\n"));
 }
 
 static void
 help_set_config(void)
 {
 	printf(_("%s set-config -B backup-dir --instance=instance_name\n"), PROGRAM_NAME);
-	printf(_("                 [--log-level=log-level]\n"));
+	printf(_("                 [--log-level-console=log-level-console]\n"));
+	printf(_("                 [--log-level-file=log-level-file]\n"));
 	printf(_("                 [--log-filename=log-filename]\n"));
 	printf(_("                 [--error-log-filename=error-log-filename]\n"));
 	printf(_("                 [--log-directory=log-directory]\n"));
@@ -276,7 +273,10 @@ help_set_config(void)
 	printf(_("      --instance=instance_name     name of the instance\n"));
 
 	printf(_("\n  Logging options:\n"));
-	printf(_("      --log-level=log-level        controls which message levels are sent to the log\n"));
+	printf(_("      --log-level-console=log-level-console\n"
+			 "                                   controls which message levels are sent to the stderr\n"));
+	printf(_("      --log-level-file=log-level-file\n"
+			 "                                   controls which message levels are sent to a log file\n"));
 	printf(_("      --log-filename=log-filename  file names of the created log files which is treated as as strftime pattern\n"));
 	printf(_("      --error-log-filename=error-log-filename\n"));
 	printf(_("                                   file names of the created log files for error messages\n"));

src/pg_probackup.c

@@ -93,7 +93,8 @@ ProbackupSubcmd backup_subcmd;
 bool		help = false;
 
 static void opt_backup_mode(pgut_option *opt, const char *arg);
-static void opt_log_level(pgut_option *opt, const char *arg);
+static void opt_log_level_console(pgut_option *opt, const char *arg);
+static void opt_log_level_file(pgut_option *opt, const char *arg);
 static void opt_compress_alg(pgut_option *opt, const char *arg);
 
 static pgut_option options[] =
@@ -141,13 +142,13 @@ static pgut_option options[] =
 	{ 'u', 137, "compress-level",		&compress_level,	SOURCE_CMDLINE },
 	{ 'b', 138, "compress",				&compress_shortcut,	SOURCE_CMDLINE },
 	/* logging options */
-	{ 'b', 'l', "log",					&log_to_file,		SOURCE_CMDLINE },
-	{ 'f', 140, "log-level",			opt_log_level,		SOURCE_CMDLINE },
-	{ 's', 141, "log-filename",			&log_filename,		SOURCE_CMDLINE },
-	{ 's', 142, "error-log-filename",	&error_log_filename, SOURCE_CMDLINE },
-	{ 's', 143, "log-directory",		&log_directory,		SOURCE_CMDLINE },
-	{ 'u', 144, "log-rotation-size",	&log_rotation_size,	SOURCE_CMDLINE },
-	{ 'u', 145, "log-rotation-age",		&log_rotation_age,	SOURCE_CMDLINE },
+	{ 'f', 140, "log-level-console",	opt_log_level_console,	SOURCE_CMDLINE },
+	{ 'f', 141, "log-level-file",		opt_log_level_file,	SOURCE_CMDLINE },
+	{ 's', 142, "log-filename",			&log_filename,		SOURCE_CMDLINE },
+	{ 's', 143, "error-log-filename",	&error_log_filename, SOURCE_CMDLINE },
+	{ 's', 144, "log-directory",		&log_directory,		SOURCE_CMDLINE },
+	{ 'u', 145, "log-rotation-size",	&log_rotation_size,	SOURCE_CMDLINE },
+	{ 'u', 146, "log-rotation-age",		&log_rotation_age,	SOURCE_CMDLINE },
 	/* connection options */
 	{ 's', 'd', "pgdatabase",			&pgut_dbname,		SOURCE_CMDLINE },
 	{ 's', 'h', "pghost",				&host,				SOURCE_CMDLINE },
@@ -422,9 +423,15 @@ opt_backup_mode(pgut_option *opt, const char *arg)
 }
 
 static void
-opt_log_level(pgut_option *opt, const char *arg)
+opt_log_level_console(pgut_option *opt, const char *arg)
 {
-	log_level = parse_log_level(arg);
+	log_level_console = parse_log_level(arg);
+}
+
+static void
+opt_log_level_file(pgut_option *opt, const char *arg)
+{
+	log_level_file = parse_log_level(arg);
 }
 
 CompressAlg

src/pg_probackup.h

@@ -165,8 +165,8 @@ typedef struct pgBackupConfig
 	const char *master_user;
 	int			replica_timeout;
 
-	int			log_to_file;
-	int			log_level;
+	int			log_level_console;
+	int			log_level_file;
 	char	   *log_filename;
 	char	   *error_log_filename;
 	char	   *log_directory;

src/restore.c

@@ -353,7 +353,7 @@ restore_backup(pgBackup *backup)
 	parray_walk(files, pgFileFree);
 	parray_free(files);
 
-	if (LOG_LEVEL <= LOG)
+	if (LOG_LEVEL_CONSOLE <= LOG || LOG_LEVEL_FILE <= LOG)
 	{
 		char	   *backup_id;
 
@@ -396,7 +396,7 @@ remove_deleted_files(pgBackup *backup)
 		if (parray_bsearch(files, file, pgFileComparePathDesc) == NULL)
 		{
 			pgFileDelete(file);
-			if (LOG_LEVEL <= LOG)
+			if (LOG_LEVEL_CONSOLE <= LOG || LOG_LEVEL_FILE <= LOG)
 				elog(LOG, "deleted %s", GetRelativePath(file->path, pgdata));
 		}
 	}
@@ -579,7 +579,7 @@ check_tablespace_mapping(pgBackup *backup)
 	pgBackupGetPath(backup, this_backup_path, lengthof(this_backup_path), NULL);
 	read_tablespace_map(links, this_backup_path);
 
-	if (LOG_LEVEL <= LOG)
+	if (LOG_LEVEL_CONSOLE <= LOG || LOG_LEVEL_FILE <= LOG)
 	{
 		char	   *backup_id;
 

src/utils/logger.c

@@ -19,8 +19,8 @@
 
 /* Logger parameters */
 
-int			log_to_file = LOGGER_NONE;
-int			log_level = LOGGER_NONE;
+int			log_level_console = LOG_NONE;
+int			log_level_file = LOG_NONE;
 
 char	   *log_filename = NULL;
 char	   *error_log_filename = NULL;
@@ -72,7 +72,7 @@ void
 init_logger(const char *root_path)
 {
 	/* Set log path */
-	if (LOG_TO_FILE || error_log_filename)
+	if (LOG_LEVEL_FILE != LOG_OFF || error_log_filename)
 	{
 		if (log_directory)
 			strcpy(log_path, log_directory);
@@ -132,20 +132,18 @@ elog_internal(int elevel, const char *fmt, va_list args)
 	time_t		log_time = (time_t) time(NULL);
 	char		strfbuf[128];
 
-	write_to_file = log_path[0] != '\0' && !logging_to_file &&
-		(LOG_TO_FILE || error_log_filename);
+	write_to_file = !logging_to_file && elevel >= LOG_LEVEL_FILE;
+	write_to_error_log = !logging_to_file &&
+		elevel >= ERROR && error_log_filename;
+	write_to_stderr = elevel >= LOG_LEVEL_CONSOLE;
 
 	/*
 	 * There is no need to lock if this is elog() from upper elog() and
 	 * logging is not initialized.
 	 */
-	if (write_to_file)
+	if (write_to_file || write_to_error_log)
 		pthread_mutex_lock(&log_file_mutex);
 
-	write_to_error_log =
-		elevel >= ERROR && error_log_filename && write_to_file;
-	write_to_stderr = elevel >= ERROR || !LOG_TO_FILE;
-
 	/* We need copy args only if we need write to error log file */
 	if (write_to_error_log)
 		va_copy(error_args, args);
@@ -156,7 +154,7 @@ elog_internal(int elevel, const char *fmt, va_list args)
 	if (write_to_stderr && write_to_file)
 		va_copy(std_args, args);
 
-	if (write_to_file)
+	if (write_to_file || write_to_error_log)
 		strftime(strfbuf, sizeof(strfbuf), "%Y-%m-%d %H:%M:%S %Z",
 				 localtime(&log_time));
 
@@ -165,7 +163,7 @@ elog_internal(int elevel, const char *fmt, va_list args)
 	 * Do not write to file if this error was raised during write previous
 	 * message.
 	 */
-	if (LOG_TO_FILE && write_to_file)
+	if (write_to_file)
 	{
 		logging_to_file = true;
 
@@ -229,7 +227,7 @@ elog_internal(int elevel, const char *fmt, va_list args)
 			va_end(std_args);
 	}
 
-	if (write_to_file)
+	if (write_to_file || write_to_error_log)
 		pthread_mutex_unlock(&log_file_mutex);
 
 	/* Exit with code if it is an error */
@@ -249,7 +247,7 @@ elog(int elevel, const char *fmt, ...)
 	 * Do not log message if severity level is less than log_level.
 	 * It is the little optimisation to put it here not in elog_internal().
 	 */
-	if (elevel < LOG_LEVEL && elevel < ERROR)
+	if (elevel < LOG_LEVEL_CONSOLE && elevel < LOG_LEVEL_FILE && elevel < ERROR)
 		return;
 
 	va_start(args, fmt);
@@ -290,7 +288,7 @@ pg_log(eLogType type, const char *fmt, ...)
 	 * Do not log message if severity level is less than log_level.
 	 * It is the little optimisation to put it here not in elog_internal().
 	 */
-	if (elevel < LOG_LEVEL && elevel < ERROR)
+	if (elevel < LOG_LEVEL_CONSOLE && elevel < LOG_LEVEL_FILE && elevel < ERROR)
 		return;
 
 	va_start(args, fmt);
@@ -315,7 +313,9 @@ parse_log_level(const char *level)
 	if (len == 0)
 		elog(ERROR, "log-level is empty");
 
-	if (pg_strncasecmp("verbose", v, len) == 0)
+	if (pg_strncasecmp("off", v, len) == 0)
+		return LOG_OFF;
+	else if (pg_strncasecmp("verbose", v, len) == 0)
 		return VERBOSE;
 	else if (pg_strncasecmp("log", v, len) == 0)
 		return LOG;
@@ -345,6 +345,8 @@ deparse_log_level(int level)
 {
 	switch (level)
 	{
+		case LOG_OFF:
+			return "OFF";
 		case VERBOSE:
 			return "VERBOSE";
 		case LOG:

src/utils/logger.h

@@ -12,7 +12,7 @@
 
 #include "postgres_fe.h"
 
-#define LOGGER_NONE	(-10)
+#define LOG_NONE	(-10)
 
 /* Log level */
 #define VERBOSE		(-5)
@@ -23,11 +23,13 @@
 #define ERROR		1
 #define FATAL		2
 #define PANIC		3
+#define LOG_OFF		10
 
 /* Logger parameters */
 
 extern int			log_to_file;
-extern int			log_level;
+extern int			log_level_console;
+extern int			log_level_file;
 
 extern char		   *log_filename;
 extern char		   *error_log_filename;
@@ -37,8 +39,8 @@ extern char			log_path[MAXPGPATH];
 extern int			log_rotation_size;
 extern int			log_rotation_age;
 
-#define LOG_TO_FILE ((log_to_file == LOGGER_NONE) ? false : (bool) log_to_file)
-#define LOG_LEVEL ((log_level == LOGGER_NONE) ? INFO : log_level)
+#define LOG_LEVEL_CONSOLE ((log_level_console == LOG_NONE) ? INFO : log_level_console)
+#define LOG_LEVEL_FILE ((log_level_file == LOG_NONE) ? LOG_OFF : log_level_file)
 
 #undef elog
 extern void elog(int elevel, const char *fmt, ...) pg_attribute_printf(2, 3);