From f5a933aa4ea7980c3df6d74d845a95f2ce0d5153 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 22 Aug 2023 15:16:28 +0200 Subject: [PATCH] Make error messages about WAL segment size more consistent In passing, make pg_controldata use the logging API for warning messages. --- src/backend/access/transam/xlog.c | 19 +++++++++++--- src/backend/bootstrap/bootstrap.c | 11 +------- src/backend/utils/misc/guc_tables.c | 2 +- src/bin/initdb/initdb.c | 25 +++++-------------- src/bin/pg_basebackup/streamutil.c | 5 ++-- src/bin/pg_controldata/pg_controldata.c | 23 ++++++++--------- .../pg_controldata/t/001_pg_controldata.pl | 6 ++--- src/bin/pg_resetwal/Makefile | 2 ++ src/bin/pg_resetwal/pg_resetwal.c | 18 +++++++------ src/bin/pg_rewind/pg_rewind.c | 12 ++++++--- src/bin/pg_waldump/pg_waldump.c | 12 ++++++--- src/include/utils/guc_hooks.h | 1 + 12 files changed, 71 insertions(+), 65 deletions(-) diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 60c0b7ec3a..f6f8adc72a 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -1995,6 +1995,18 @@ assign_checkpoint_completion_target(double newval, void *extra) CalculateCheckpointSegments(); } +bool +check_wal_segment_size(int *newval, void **extra, GucSource source) +{ + if (!IsValidWalSegSize(*newval)) + { + GUC_check_errdetail("The WAL segment size must be a power of two between 1 MB and 1 GB."); + return false; + } + + return true; +} + /* * At a checkpoint, how many WAL segments to recycle as preallocated future * XLOG segments? Returns the highest segment that should be preallocated. @@ -4145,10 +4157,11 @@ ReadControlFile(void) if (!IsValidWalSegSize(wal_segment_size)) ereport(ERROR, (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg_plural("WAL segment size must be a power of two between 1 MB and 1 GB, but the control file specifies %d byte", - "WAL segment size must be a power of two between 1 MB and 1 GB, but the control file specifies %d bytes", + errmsg_plural("invalid WAL segment size in control file (%d byte)", + "invalid WAL segment size in control file (%d bytes)", wal_segment_size, - wal_segment_size))); + wal_segment_size), + errdetail("The WAL segment size must be a power of two between 1 MB and 1 GB."))); snprintf(wal_segsz_str, sizeof(wal_segsz_str), "%d", wal_segment_size); SetConfigOption("wal_segment_size", wal_segsz_str, PGC_INTERNAL, diff --git a/src/backend/bootstrap/bootstrap.c b/src/backend/bootstrap/bootstrap.c index 49e956b2c5..c9ed649d0d 100644 --- a/src/backend/bootstrap/bootstrap.c +++ b/src/backend/bootstrap/bootstrap.c @@ -280,16 +280,7 @@ BootstrapModeMain(int argc, char *argv[], bool check_only) strlcpy(OutputFileName, optarg, MAXPGPATH); break; case 'X': - { - int WalSegSz = strtoul(optarg, NULL, 0); - - if (!IsValidWalSegSize(WalSegSz)) - ereport(ERROR, - (errcode(ERRCODE_INVALID_PARAMETER_VALUE), - errmsg("-X requires a power of two value between 1 MB and 1 GB"))); - SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL, - PGC_S_DYNAMIC_DEFAULT); - } + SetConfigOption("wal_segment_size", optarg, PGC_INTERNAL, PGC_S_DYNAMIC_DEFAULT); break; default: write_stderr("Try \"%s --help\" for more information.\n", diff --git a/src/backend/utils/misc/guc_tables.c b/src/backend/utils/misc/guc_tables.c index f9dba43b8c..f14656c1e2 100644 --- a/src/backend/utils/misc/guc_tables.c +++ b/src/backend/utils/misc/guc_tables.c @@ -3166,7 +3166,7 @@ struct config_int ConfigureNamesInt[] = DEFAULT_XLOG_SEG_SIZE, WalSegMinSize, WalSegMaxSize, - NULL, NULL, NULL + check_wal_segment_size, NULL, NULL }, { diff --git a/src/bin/initdb/initdb.c b/src/bin/initdb/initdb.c index 3f4167682a..c66467eb95 100644 --- a/src/bin/initdb/initdb.c +++ b/src/bin/initdb/initdb.c @@ -76,6 +76,7 @@ #include "common/restricted_token.h" #include "common/string.h" #include "common/username.h" +#include "fe_utils/option_utils.h" #include "fe_utils/string_utils.h" #include "getopt_long.h" #include "mb/pg_wchar.h" @@ -163,8 +164,7 @@ static bool sync_only = false; static bool show_setting = false; static bool data_checksums = false; static char *xlog_dir = NULL; -static char *str_wal_segment_size_mb = NULL; -static int wal_segment_size_mb; +static int wal_segment_size_mb = (DEFAULT_XLOG_SEG_SIZE) / (1024 * 1024); /* internal vars */ @@ -3258,7 +3258,8 @@ main(int argc, char *argv[]) xlog_dir = pg_strdup(optarg); break; case 12: - str_wal_segment_size_mb = pg_strdup(optarg); + if (!option_parse_int(optarg, "--wal-segsize", 1, 1024, &wal_segment_size_mb)) + exit(1); break; case 13: noinstructions = true; @@ -3348,22 +3349,8 @@ main(int argc, char *argv[]) check_need_password(authmethodlocal, authmethodhost); - /* set wal segment size */ - if (str_wal_segment_size_mb == NULL) - wal_segment_size_mb = (DEFAULT_XLOG_SEG_SIZE) / (1024 * 1024); - else - { - char *endptr; - - /* check that the argument is a number */ - wal_segment_size_mb = strtol(str_wal_segment_size_mb, &endptr, 10); - - /* verify that wal segment size is valid */ - if (endptr == str_wal_segment_size_mb || *endptr != '\0') - pg_fatal("argument of --wal-segsize must be a number"); - if (!IsValidWalSegSize(wal_segment_size_mb * 1024 * 1024)) - pg_fatal("argument of --wal-segsize must be a power of 2 between 1 and 1024"); - } + if (!IsValidWalSegSize(wal_segment_size_mb * 1024 * 1024)) + pg_fatal("argument of %s must be a power of 2 between 1 and 1024", "--wal-segsize"); get_restricted_token(); diff --git a/src/bin/pg_basebackup/streamutil.c b/src/bin/pg_basebackup/streamutil.c index 15514599c4..75ab9e56f3 100644 --- a/src/bin/pg_basebackup/streamutil.c +++ b/src/bin/pg_basebackup/streamutil.c @@ -321,10 +321,11 @@ RetrieveWalSegSize(PGconn *conn) if (!IsValidWalSegSize(WalSegSz)) { - pg_log_error(ngettext("WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server reported a value of %d byte", - "WAL segment size must be a power of two between 1 MB and 1 GB, but the remote server reported a value of %d bytes", + pg_log_error(ngettext("remote server reported invalid WAL segment size (%d byte)", + "remote server reported invalid WAL segment size (%d bytes)", WalSegSz), WalSegSz); + pg_log_error_detail("The WAL segment size must be a power of two between 1 MB and 1 GB."); return false; } diff --git a/src/bin/pg_controldata/pg_controldata.c b/src/bin/pg_controldata/pg_controldata.c index c390ec51ce..93e0837947 100644 --- a/src/bin/pg_controldata/pg_controldata.c +++ b/src/bin/pg_controldata/pg_controldata.c @@ -167,24 +167,23 @@ main(int argc, char *argv[]) /* get a copy of the control file */ ControlFile = get_controlfile(DataDir, &crc_ok); if (!crc_ok) - printf(_("WARNING: Calculated CRC checksum does not match value stored in file.\n" - "Either the file is corrupt, or it has a different layout than this program\n" - "is expecting. The results below are untrustworthy.\n\n")); + { + pg_log_warning("calculated CRC checksum does not match value stored in control file"); + pg_log_warning_detail("Either the control file is corrupt, or it has a different layout than this program " + "is expecting. The results below are untrustworthy."); + } /* set wal segment size */ WalSegSz = ControlFile->xlog_seg_size; if (!IsValidWalSegSize(WalSegSz)) { - printf(_("WARNING: invalid WAL segment size\n")); - printf(ngettext("The WAL segment size stored in the file, %d byte, is not a power of two\n" - "between 1 MB and 1 GB. The file is corrupt and the results below are\n" - "untrustworthy.\n\n", - "The WAL segment size stored in the file, %d bytes, is not a power of two\n" - "between 1 MB and 1 GB. The file is corrupt and the results below are\n" - "untrustworthy.\n\n", - WalSegSz), - WalSegSz); + pg_log_warning(ngettext("invalid WAL segment size in control file (%d byte)", + "invalid WAL segment size in control file (%d bytes)", + WalSegSz), + WalSegSz); + pg_log_warning_detail("The WAL segment size must be a power of two between 1 MB and 1 GB."); + pg_log_warning_detail("The file is corrupt and the results below are untrustworthy."); } /* diff --git a/src/bin/pg_controldata/t/001_pg_controldata.pl b/src/bin/pg_controldata/t/001_pg_controldata.pl index 0c641036e9..4918ea8944 100644 --- a/src/bin/pg_controldata/t/001_pg_controldata.pl +++ b/src/bin/pg_controldata/t/001_pg_controldata.pl @@ -36,11 +36,11 @@ command_checks_all( [ 'pg_controldata', $node->data_dir ], 0, + [qr/./], [ - qr/WARNING: Calculated CRC checksum does not match value stored in file/, - qr/WARNING: invalid WAL segment size/ + qr/warning: calculated CRC checksum does not match value stored in control file/, + qr/warning: invalid WAL segment size/ ], - [qr/^$/], 'pg_controldata with corrupted pg_control'); done_testing(); diff --git a/src/bin/pg_resetwal/Makefile b/src/bin/pg_resetwal/Makefile index a363b948b5..5c86435e22 100644 --- a/src/bin/pg_resetwal/Makefile +++ b/src/bin/pg_resetwal/Makefile @@ -15,6 +15,8 @@ subdir = src/bin/pg_resetwal top_builddir = ../../.. include $(top_builddir)/src/Makefile.global +LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils + OBJS = \ $(WIN32RES) \ pg_resetwal.o diff --git a/src/bin/pg_resetwal/pg_resetwal.c b/src/bin/pg_resetwal/pg_resetwal.c index e7ef2b8bd0..9bebc2a995 100644 --- a/src/bin/pg_resetwal/pg_resetwal.c +++ b/src/bin/pg_resetwal/pg_resetwal.c @@ -55,6 +55,7 @@ #include "common/logging.h" #include "common/restricted_token.h" #include "common/string.h" +#include "fe_utils/option_utils.h" #include "getopt_long.h" #include "pg_getopt.h" #include "storage/large_object.h" @@ -290,13 +291,16 @@ main(int argc, char *argv[]) break; case 1: - errno = 0; - set_wal_segsize = strtol(optarg, &endptr, 10) * 1024 * 1024; - if (endptr == optarg || *endptr != '\0' || errno != 0) - pg_fatal("argument of --wal-segsize must be a number"); - if (!IsValidWalSegSize(set_wal_segsize)) - pg_fatal("argument of --wal-segsize must be a power of 2 between 1 and 1024"); - break; + { + int wal_segsize_mb; + + if (!option_parse_int(optarg, "--wal-segsize", 1, 1024, &wal_segsize_mb)) + exit(1); + set_wal_segsize = wal_segsize_mb * 1024 * 1024; + if (!IsValidWalSegSize(set_wal_segsize)) + pg_fatal("argument of %s must be a power of 2 between 1 and 1024", "--wal-segsize"); + break; + } default: /* getopt_long already emitted a complaint */ diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c index f7f3b8227f..7f69f02441 100644 --- a/src/bin/pg_rewind/pg_rewind.c +++ b/src/bin/pg_rewind/pg_rewind.c @@ -1023,10 +1023,14 @@ digestControlFile(ControlFileData *ControlFile, const char *content, WalSegSz = ControlFile->xlog_seg_size; if (!IsValidWalSegSize(WalSegSz)) - pg_fatal(ngettext("WAL segment size must be a power of two between 1 MB and 1 GB, but the control file specifies %d byte", - "WAL segment size must be a power of two between 1 MB and 1 GB, but the control file specifies %d bytes", - WalSegSz), - WalSegSz); + { + pg_log_error(ngettext("invalid WAL segment size in control file (%d byte)", + "invalid WAL segment size in control file (%d bytes)", + WalSegSz), + WalSegSz); + pg_log_error_detail("The WAL segment size must be a power of two between 1 MB and 1 GB."); + exit(1); + } /* Additional checks on control file */ checkControlFile(ControlFile); diff --git a/src/bin/pg_waldump/pg_waldump.c b/src/bin/pg_waldump/pg_waldump.c index e8b5a6cd61..b9acfed3b7 100644 --- a/src/bin/pg_waldump/pg_waldump.c +++ b/src/bin/pg_waldump/pg_waldump.c @@ -252,10 +252,14 @@ search_directory(const char *directory, const char *fname) WalSegSz = longhdr->xlp_seg_size; if (!IsValidWalSegSize(WalSegSz)) - pg_fatal(ngettext("WAL segment size must be a power of two between 1 MB and 1 GB, but the WAL file \"%s\" header specifies %d byte", - "WAL segment size must be a power of two between 1 MB and 1 GB, but the WAL file \"%s\" header specifies %d bytes", - WalSegSz), - fname, WalSegSz); + { + pg_log_error(ngettext("invalid WAL segment size in WAL file \"%s\" (%d byte)", + "invalid WAL segment size in WAL file \"%s\" (%d bytes)", + WalSegSz), + fname, WalSegSz); + pg_log_error_detail("The WAL segment size must be a power of two between 1 MB and 1 GB."); + exit(1); + } } else if (r < 0) pg_fatal("could not read file \"%s\": %m", diff --git a/src/include/utils/guc_hooks.h b/src/include/utils/guc_hooks.h index 2ecb9fc086..5d8b2c981b 100644 --- a/src/include/utils/guc_hooks.h +++ b/src/include/utils/guc_hooks.h @@ -156,6 +156,7 @@ extern bool check_wal_buffers(int *newval, void **extra, GucSource source); extern bool check_wal_consistency_checking(char **newval, void **extra, GucSource source); extern void assign_wal_consistency_checking(const char *newval, void *extra); +extern bool check_wal_segment_size(int *newval, void **extra, GucSource source); extern void assign_xlog_sync_method(int new_sync_method, void *extra); extern bool check_io_direct(char **newval, void **extra, GucSource source); extern void assign_io_direct(const char *newval, void *extra); -- 2.41.0