Alvaro Herrera <alvhe...@2ndquadrant.com> writes:
> On 2019-May-13, Tom Lane wrote:
>> I started working on a patch to do that, and soon noticed that there
>> are pre-existing files logging.[hc] in src/bin/pg_rewind/. This seems
>> like a Bad Thing, in fact the #includes in pg_rewind/ are already a
>> little confused due to this. I think we should either rename those
>> two pg_rewind files to something else, or rename the generic ones,
>> perhaps to "fe_logging.[hc]". The latter could be done nearly
>> trivially as part of the movement patch, but on cosmetic grounds
>> I'd be more inclined to do the former instead. Thoughts?
> I'd rename both :-)
On closer inspection, there's so little left in pg_rewind's logging.h/.c
(one function and a couple of global variables) that the better answer
is probably just to move those objects somewhere else and nuke the
separate files altogether. As attached.
regards, tom lane
diff --git a/src/bin/pg_rewind/Makefile b/src/bin/pg_rewind/Makefile
index 019e199..5455fd4 100644
--- a/src/bin/pg_rewind/Makefile
+++ b/src/bin/pg_rewind/Makefile
@@ -19,7 +19,7 @@ override CPPFLAGS := -I$(libpq_srcdir) -DFRONTEND $(CPPFLAGS)
LDFLAGS_INTERNAL += -L$(top_builddir)/src/fe_utils -lpgfeutils $(libpq_pgport)
OBJS = pg_rewind.o parsexlog.o xlogreader.o datapagemap.o timeline.o \
- fetch.o file_ops.o copy_fetch.o libpq_fetch.o filemap.o logging.o \
+ fetch.o file_ops.o copy_fetch.o libpq_fetch.o filemap.o \
$(WIN32RES)
EXTRA_CLEAN = xlogreader.c
diff --git a/src/bin/pg_rewind/copy_fetch.c b/src/bin/pg_rewind/copy_fetch.c
index 3fd0404..2ada861 100644
--- a/src/bin/pg_rewind/copy_fetch.c
+++ b/src/bin/pg_rewind/copy_fetch.c
@@ -18,7 +18,6 @@
#include "fetch.h"
#include "file_ops.h"
#include "filemap.h"
-#include "logging.h"
#include "pg_rewind.h"
static void recurse_dir(const char *datadir, const char *path,
diff --git a/src/bin/pg_rewind/file_ops.c b/src/bin/pg_rewind/file_ops.c
index e442f93..f9e41b1 100644
--- a/src/bin/pg_rewind/file_ops.c
+++ b/src/bin/pg_rewind/file_ops.c
@@ -21,7 +21,6 @@
#include "common/file_perm.h"
#include "file_ops.h"
#include "filemap.h"
-#include "logging.h"
#include "pg_rewind.h"
/*
diff --git a/src/bin/pg_rewind/filemap.c b/src/bin/pg_rewind/filemap.c
index 63d0bae..813eadc 100644
--- a/src/bin/pg_rewind/filemap.c
+++ b/src/bin/pg_rewind/filemap.c
@@ -15,12 +15,10 @@
#include "datapagemap.h"
#include "filemap.h"
-#include "logging.h"
#include "pg_rewind.h"
#include "common/string.h"
#include "catalog/pg_tablespace_d.h"
-#include "fe_utils/logging.h"
#include "storage/fd.h"
filemap_t *filemap = NULL;
diff --git a/src/bin/pg_rewind/libpq_fetch.c b/src/bin/pg_rewind/libpq_fetch.c
index 11ec045..b6fa7e5 100644
--- a/src/bin/pg_rewind/libpq_fetch.c
+++ b/src/bin/pg_rewind/libpq_fetch.c
@@ -19,12 +19,10 @@
#include "fetch.h"
#include "file_ops.h"
#include "filemap.h"
-#include "logging.h"
#include "libpq-fe.h"
#include "catalog/pg_type_d.h"
#include "fe_utils/connect.h"
-#include "fe_utils/logging.h"
#include "port/pg_bswap.h"
static PGconn *conn = NULL;
diff --git a/src/bin/pg_rewind/logging.c b/src/bin/pg_rewind/logging.c
deleted file mode 100644
index 8169f73..0000000
--- a/src/bin/pg_rewind/logging.c
+++ /dev/null
@@ -1,79 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * logging.c
- * logging functions
- *
- * Copyright (c) 2010-2019, PostgreSQL Global Development Group
- *
- *-------------------------------------------------------------------------
- */
-#include "postgres_fe.h"
-
-#include <unistd.h>
-#include <time.h>
-
-#include "pg_rewind.h"
-#include "logging.h"
-
-#include "pgtime.h"
-
-/* Progress counters */
-uint64 fetch_size;
-uint64 fetch_done;
-
-static pg_time_t last_progress_report = 0;
-
-
-/*
- * Print a progress report based on the global variables.
- *
- * Progress report is written at maximum once per second, unless the
- * force parameter is set to true.
- */
-void
-progress_report(bool force)
-{
- int percent;
- char fetch_done_str[32];
- char fetch_size_str[32];
- pg_time_t now;
-
- if (!showprogress)
- return;
-
- now = time(NULL);
- if (now == last_progress_report && !force)
- return; /* Max once per second */
-
- last_progress_report = now;
- percent = fetch_size ? (int) ((fetch_done) * 100 / fetch_size) : 0;
-
- /*
- * Avoid overflowing past 100% or the full size. This may make the total
- * size number change as we approach the end of the backup (the estimate
- * will always be wrong if WAL is included), but that's better than having
- * the done column be bigger than the total.
- */
- if (percent > 100)
- percent = 100;
- if (fetch_done > fetch_size)
- fetch_size = fetch_done;
-
- /*
- * Separate step to keep platform-dependent format code out of
- * translatable strings. And we only test for INT64_FORMAT availability
- * in snprintf, not fprintf.
- */
- snprintf(fetch_done_str, sizeof(fetch_done_str), INT64_FORMAT,
- fetch_done / 1024);
- snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
- fetch_size / 1024);
-
- fprintf(stderr, _("%*s/%s kB (%d%%) copied"),
- (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
- percent);
- if (isatty(fileno(stderr)))
- fprintf(stderr, "\r");
- else
- fprintf(stderr, "\n");
-}
diff --git a/src/bin/pg_rewind/logging.h b/src/bin/pg_rewind/logging.h
deleted file mode 100644
index 81e17ac..0000000
--- a/src/bin/pg_rewind/logging.h
+++ /dev/null
@@ -1,25 +0,0 @@
-/*-------------------------------------------------------------------------
- *
- * logging.h
- * prototypes for logging functions
- *
- *
- * Portions Copyright (c) 1996-2019, PostgreSQL Global Development Group
- * Portions Copyright (c) 1994, Regents of the University of California
- *
- *-------------------------------------------------------------------------
- */
-#ifndef PG_REWIND_LOGGING_H
-#define PG_REWIND_LOGGING_H
-
-#include "fe_utils/logging.h"
-
-/* progress counters */
-extern uint64 fetch_size;
-extern uint64 fetch_done;
-
-extern void progress_report(bool force);
-
-#define pg_fatal(...) do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
-
-#endif /* PG_REWIND_LOGGING_H */
diff --git a/src/bin/pg_rewind/nls.mk b/src/bin/pg_rewind/nls.mk
index 58c9ede..ba29d62 100644
--- a/src/bin/pg_rewind/nls.mk
+++ b/src/bin/pg_rewind/nls.mk
@@ -1,7 +1,7 @@
# src/bin/pg_rewind/nls.mk
CATALOG_NAME = pg_rewind
AVAIL_LANGUAGES =de es fr it ja ko pl pt_BR ru sv tr zh_CN
-GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) copy_fetch.c datapagemap.c fetch.c file_ops.c filemap.c libpq_fetch.c logging.c parsexlog.c pg_rewind.c timeline.c ../../common/fe_memutils.c ../../common/restricted_token.c xlogreader.c
+GETTEXT_FILES = $(FRONTEND_COMMON_GETTEXT_FILES) copy_fetch.c datapagemap.c fetch.c file_ops.c filemap.c libpq_fetch.c parsexlog.c pg_rewind.c timeline.c ../../common/fe_memutils.c ../../common/restricted_token.c xlogreader.c
GETTEXT_TRIGGERS = $(FRONTEND_COMMON_GETTEXT_TRIGGERS) pg_fatal report_invalid_record:2
GETTEXT_FLAGS = $(FRONTEND_COMMON_GETTEXT_FLAGS) \
pg_fatal:1:c-format \
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index 7709b96..65e523f 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -15,7 +15,6 @@
#include "pg_rewind.h"
#include "filemap.h"
-#include "logging.h"
#include "access/rmgr.h"
#include "access/xlog_internal.h"
diff --git a/src/bin/pg_rewind/pg_rewind.c b/src/bin/pg_rewind/pg_rewind.c
index 3702efa..29a0eb5 100644
--- a/src/bin/pg_rewind/pg_rewind.c
+++ b/src/bin/pg_rewind/pg_rewind.c
@@ -18,7 +18,6 @@
#include "fetch.h"
#include "file_ops.h"
#include "filemap.h"
-#include "logging.h"
#include "access/timeline.h"
#include "access/xlog_internal.h"
@@ -28,7 +27,6 @@
#include "common/file_perm.h"
#include "common/file_utils.h"
#include "common/restricted_token.h"
-#include "fe_utils/logging.h"
#include "getopt_long.h"
#include "storage/bufpage.h"
@@ -63,6 +61,12 @@ bool do_sync = true;
TimeLineHistoryEntry *targetHistory;
int targetNentries;
+/* Progress counters */
+uint64 fetch_size;
+uint64 fetch_done;
+static pg_time_t last_progress_report = 0;
+
+
static void
usage(const char *progname)
{
@@ -446,6 +450,60 @@ sanityChecks(void)
}
/*
+ * Print a progress report based on the global variables.
+ *
+ * Progress report is written at maximum once per second, unless the
+ * force parameter is set to true.
+ */
+void
+progress_report(bool force)
+{
+ int percent;
+ char fetch_done_str[32];
+ char fetch_size_str[32];
+ pg_time_t now;
+
+ if (!showprogress)
+ return;
+
+ now = time(NULL);
+ if (now == last_progress_report && !force)
+ return; /* Max once per second */
+
+ last_progress_report = now;
+ percent = fetch_size ? (int) ((fetch_done) * 100 / fetch_size) : 0;
+
+ /*
+ * Avoid overflowing past 100% or the full size. This may make the total
+ * size number change as we approach the end of the backup (the estimate
+ * will always be wrong if WAL is included), but that's better than having
+ * the done column be bigger than the total.
+ */
+ if (percent > 100)
+ percent = 100;
+ if (fetch_done > fetch_size)
+ fetch_size = fetch_done;
+
+ /*
+ * Separate step to keep platform-dependent format code out of
+ * translatable strings. And we only test for INT64_FORMAT availability
+ * in snprintf, not fprintf.
+ */
+ snprintf(fetch_done_str, sizeof(fetch_done_str), INT64_FORMAT,
+ fetch_done / 1024);
+ snprintf(fetch_size_str, sizeof(fetch_size_str), INT64_FORMAT,
+ fetch_size / 1024);
+
+ fprintf(stderr, _("%*s/%s kB (%d%%) copied"),
+ (int) strlen(fetch_size_str), fetch_done_str, fetch_size_str,
+ percent);
+ if (isatty(fileno(stderr)))
+ fprintf(stderr, "\r");
+ else
+ fprintf(stderr, "\n");
+}
+
+/*
* Find minimum from two WAL locations assuming InvalidXLogRecPtr means
* infinity as src/include/access/timeline.h states. This routine should
* be used only when comparing WAL locations related to history files.
diff --git a/src/bin/pg_rewind/pg_rewind.h b/src/bin/pg_rewind/pg_rewind.h
index f2cc280..9e6f947 100644
--- a/src/bin/pg_rewind/pg_rewind.h
+++ b/src/bin/pg_rewind/pg_rewind.h
@@ -17,6 +17,8 @@
#include "storage/block.h"
#include "storage/relfilenode.h"
+#include "fe_utils/logging.h"
+
/* Configuration options */
extern char *datadir_target;
extern char *datadir_source;
@@ -29,6 +31,13 @@ extern int WalSegSz;
extern TimeLineHistoryEntry *targetHistory;
extern int targetNentries;
+/* Progress counters */
+extern uint64 fetch_size;
+extern uint64 fetch_done;
+
+/* logging support */
+#define pg_fatal(...) do { pg_log_fatal(__VA_ARGS__); exit(1); } while(0)
+
/* in parsexlog.c */
extern void extractPageMap(const char *datadir, XLogRecPtr startpoint,
int tliIndex, XLogRecPtr endpoint);
@@ -39,6 +48,9 @@ extern void findLastCheckpoint(const char *datadir, XLogRecPtr searchptr,
extern XLogRecPtr readOneRecord(const char *datadir, XLogRecPtr ptr,
int tliIndex);
+/* in pg_rewind.c */
+extern void progress_report(bool force);
+
/* in timeline.c */
extern TimeLineHistoryEntry *rewind_parseTimeLineHistory(char *buffer,
TimeLineID targetTLI, int *nentries);
diff --git a/src/bin/pg_rewind/timeline.c b/src/bin/pg_rewind/timeline.c
index 12b19b3..987452c 100644
--- a/src/bin/pg_rewind/timeline.c
+++ b/src/bin/pg_rewind/timeline.c
@@ -13,7 +13,6 @@
#include "access/timeline.h"
#include "access/xlog_internal.h"
-#include "fe_utils/logging.h"
/*
* This is copy-pasted from the backend readTimeLineHistory, modified to
