Re: Hot standby, recovery infra - Mailing list pgsql-hackers

Hi,

On Wed, Jan 28, 2009 at 7:04 PM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.com> wrote:
> I've been reviewing and massaging the so called recovery infra patch.

Great!

> I feel quite good about this patch now. Given the amount of code churn, it
> requires testing, and I'll read it through one more time after sleeping over
> it. Simon, do you see anything wrong with this?

I also read this patch and found something odd. I apologize if I misread it..

> @@ -507,7 +550,7 @@ CheckArchiveTimeout(void)
>      pg_time_t    now;
>      pg_time_t    last_time;
>
> -    if (XLogArchiveTimeout <= 0)
> +    if (XLogArchiveTimeout <= 0 || !IsRecoveryProcessingMode())

The above change destroys archive_timeout because checking the timeout
is always skipped after recovery is done.

> @@ -355,6 +359,27 @@ BackgroundWriterMain(void)
>       */
>      PG_SETMASK(&UnBlockSig);
>
> +    BgWriterRecoveryMode = IsRecoveryProcessingMode();
> +
> +    if (BgWriterRecoveryMode)
> +        elog(DEBUG1, "bgwriter starting during recovery");
> +    else
> +        InitXLOGAccess();

Why is InitXLOGAccess() called also here when bgwriter is started after
recovery? That is already called by AuxiliaryProcessMain().

> @@ -1302,7 +1314,7 @@ ServerLoop(void)
>           * state that prevents it, start one.  It doesn't matter if this
>           * fails, we'll just try again later.
>           */
> -        if (BgWriterPID == 0 && pmState == PM_RUN)
> +        if (BgWriterPID == 0 && (pmState == PM_RUN || pmState == PM_RECOVERY))
>              BgWriterPID = StartBackgroundWriter();

Likewise, we should try to start also the stats collector during recovery?

> @@ -2103,7 +2148,8 @@ XLogFileInit(uint32 log, uint32 seg,
>          unlink(tmppath);
>      }
>
> -    elog(DEBUG2, "done creating and filling new WAL file");
> +    XLogFileName(tmppath, ThisTimeLineID, log, seg);
> +    elog(DEBUG2, "done creating and filling new WAL file %s", tmppath);

This debug message is somewhat confusing, because the WAL file
represented as "tmppath" might have been already created by
previous XLogFileInit() via InstallXLogFileSegment().

regards,

-- 
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center