On 2019-May-30, Michael Paquier wrote:
> On Wed, May 29, 2019 at 12:09:08PM -0400, Alvaro Herrera wrote:
> > Are there objections to doing that now on the master branch?
>
> Adding the flush call just on HEAD is fine for me. Not sure that
> there is an actual reason to back-patch that.
Okay ... I did that (patch attached), and while my new __gcov_flush()
shows as covered after I run the src/test/recovery tests, the function I
mentioned earlier (validateRecoveryParameters) is not any more covered
after the patch than it was before. So I'm not sure how useful this
really is. Maybe someone can point to something that this patch is
doing wrong ... or maybe I'm just looking at the wrong report, or this
is not the function that we wanted to add coverage for?
(On a whim I named the symbol USE_GCOV_COVERAGE because we could
theoretically have coverage reports using some other symbol. I suppose
it could very well be just USE_COVERAGE instead.)
gcov after patch:
-: 5379:static void
100: 5380:validateRecoveryParameters(void)
-: 5381:{
100: 5382: if (!ArchiveRecoveryRequested)
81: 5383: return;
-: 5384:
-: 5385: /*
-: 5386: * Check for compulsory parameters
-: 5387: */
19: 5388: if (StandbyModeRequested)
-: 5389: {
19: 5390: if ((PrimaryConnInfo == NULL ||
strcmp(PrimaryConnInfo, "") == 0) &&
#####: 5391: (recoveryRestoreCommand == NULL ||
strcmp(recoveryRestoreCommand, "") == 0))
#####: 5392: ereport(WARNING,
-: 5393: (errmsg("specified
neither primary_conninfo nor restore_command"),
-: 5394: errhint("The database
server will regularly poll the pg_wal subdirectory to check for files placed
there.")));
-: 5395: }
-: 5396: else
-: 5397: {
#####: 5398: if (recoveryRestoreCommand == NULL ||
#####: 5399: strcmp(recoveryRestoreCommand, "") == 0)
#####: 5400: ereport(FATAL,
-: 5401:
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-: 5402: errmsg("must specify
restore_command when standby mode is not enabled")));
-: 5403: }
-: 5404:
-: 5405: /*
-: 5406: * Override any inconsistent requests. Note that this
is a change of
-: 5407: * behaviour in 9.5; prior to this we simply ignored a
request to pause if
-: 5408: * hot_standby = off, which was surprising behaviour.
-: 5409: */
38: 5410: if (recoveryTargetAction ==
RECOVERY_TARGET_ACTION_PAUSE &&
19: 5411: !EnableHotStandby)
#####: 5412: recoveryTargetAction =
RECOVERY_TARGET_ACTION_SHUTDOWN;
-: 5413:
-: 5414: /*
-: 5415: * If user specified recovery_target_timeline, validate
it or compute the
-: 5416: * "latest" value. We can't do this until after we've
gotten the restore
-: 5417: * command and set InArchiveRecovery, because we need
to fetch timeline
-: 5418: * history files from the archive.
-: 5419: */
19: 5420: if (recoveryTargetTimeLineGoal ==
RECOVERY_TARGET_TIMELINE_NUMERIC)
-: 5421: {
#####: 5422: TimeLineID rtli =
recoveryTargetTLIRequested;
-: 5423:
-: 5424: /* Timeline 1 does not have a history file, all
else should */
#####: 5425: if (rtli != 1 && !existsTimeLineHistory(rtli))
#####: 5426: ereport(FATAL,
-: 5427:
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-: 5428: errmsg("recovery
target timeline %u does not exist",
-: 5429: rtli)));
#####: 5430: recoveryTargetTLI = rtli;
-: 5431: }
19: 5432: else if (recoveryTargetTimeLineGoal ==
RECOVERY_TARGET_TIMELINE_LATEST)
-: 5433: {
-: 5434: /* We start the "latest" search from
pg_control's timeline */
19: 5435: recoveryTargetTLI =
findNewestTimeLine(recoveryTargetTLI);
-: 5436: }
-: 5437: else
-: 5438: {
-: 5439: /*
-: 5440: * else we just use the recoveryTargetTLI as
already read from
-: 5441: * ControlFile
-: 5442: */
#####: 5443: Assert(recoveryTargetTimeLineGoal ==
RECOVERY_TARGET_TIMELINE_CONTROLFILE);
-: 5444: }
-: 5445:}
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
diff --git a/configure b/configure
index fd61bf6472..c0cf19d662 100755
--- a/configure
+++ b/configure
@@ -3516,6 +3516,10 @@ fi
if test -z "$GENHTML"; then
as_fn_error $? "genhtml not found" "$LINENO" 5
fi
+
+$as_echo "#define USE_GCOV_COVERAGE 1" >>confdefs.h
+
+
;;
no)
:
diff --git a/configure.in b/configure.in
index 4586a1716c..21465bbaa6 100644
--- a/configure.in
+++ b/configure.in
@@ -222,7 +222,9 @@ fi
PGAC_PATH_PROGS(GENHTML, genhtml)
if test -z "$GENHTML"; then
AC_MSG_ERROR([genhtml not found])
-fi])
+fi
+AC_DEFINE([USE_GCOV_COVERAGE], 1, [Define to use gcov coverage support stuff])
+])
AC_SUBST(enable_coverage)
#
diff --git a/src/backend/tcop/postgres.c b/src/backend/tcop/postgres.c
index 44a59e1d4f..a483eba454 100644
--- a/src/backend/tcop/postgres.c
+++ b/src/backend/tcop/postgres.c
@@ -2729,6 +2729,14 @@ quickdie(SIGNAL_ARGS)
errhint("In a moment you should be able to reconnect to the"
" database and repeat your command.")));
+#ifdef USE_GCOV_COVERAGE
+ /*
+ * We still want to flush coverage data down to disk, which gcov's atexit
+ * callback would do, but we're preventing that below.
+ */
+ __gcov_flush();
+#endif
+
/*
* We DO NOT want to run proc_exit() or atexit() callbacks -- we're here
* because shared memory may be corrupted, so we don't want to try to
diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in
index 6cd4cfed0a..5fb1a545ed 100644
--- a/src/include/pg_config.h.in
+++ b/src/include/pg_config.h.in
@@ -916,6 +916,9 @@
(--enable-float8-byval) */
#undef USE_FLOAT8_BYVAL
+/* Define to use gcov coverage support stuff */
+#undef USE_GCOV_COVERAGE
+
/* Define to build with ICU support. (--with-icu) */
#undef USE_ICU
