Re: incorrect xlog.c coverage report

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