Re: [COMMITTERS] pgsql: Change how first WAL segment on new timeline after promotion is

From: Heikki Linnakangas <hlinnakangas(at)vmware(dot)com>
To: Andres Freund <andres(at)2ndquadrant(dot)com>
Cc: PostgreSQL-development <pgsql-hackers(at)postgreSQL(dot)org>
Subject: Re: [COMMITTERS] pgsql: Change how first WAL segment on new timeline after promotion is
Date: 2015-01-05 16:45:22
Message-ID: 54AABFA2.3050409@vmware.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-committers pgsql-hackers

On 01/03/2015 08:59 PM, Andres Freund wrote:
> Hi Heikki,
>
> While writing a test script for
> http://archives.postgresql.org/message-id/20141205002854.GE21964%40awork2.anarazel.de
> I noticed that this commit broke starting a pg_basebackup -X * without a
> recovery.conf present. Which might not be the best idea, but imo is a
> perfectly valid thing to do.
>
> To me the changes to StartupXLOG() in that commit look a bit bogus. The
> new startLogSegNo is initialized to XLByteToSeg(EndOfLog)? Which points
> to the end of the record +1? Which thus isn't guaranteed to exist as a
> segment (e.g. never if the last record was a XLOG_SWITCH).

Ah, good point.

> Did you perhaps intend to use XLogFileInit(use_existing = true)
> instead of XLogFileOpen()? That works for me.

Hmm, that doesn't sound right either. XLogFileInit is used when you
switch to a new segment, not to open an old segment for writing. It
happens to work, because with use_existing = true it will in fact always
open the old segment, instead of creating a new one, but I don't think
that's in the spirit of how that function's intended to be used.

A very simple fix is to not try opening the segment at all. There isn't
actually any requirement to have the segment open at that point,
XLogWrite() will open it the first time the WAL is flushed. The WAL is
flushed after writing the initial checkpoint or end-of-recovery record,
which happens pretty soon anyway. Any objections to the attached?

> I've attached my preliminary testscript (note it's really not that
> interesting at this point) that reliably reproduces the problem for me.

Thanks!

- Heikki

Attachment Content-Type Size
fix-first-segment-open-1.patch text/x-diff 1023 bytes

In response to

Responses

Browse pgsql-committers by date

  From Date Subject
Next Message Tom Lane 2015-01-06 00:27:23 pgsql: Fix broken pg_dump code for dumping comments on event triggers.
Previous Message Andres Freund 2015-01-05 11:54:28 pgsql: Fix oversight in recent pg_basebackup fix causing pg_receivexlog

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2015-01-05 16:49:59 Re: Additional role attributes && superuser review
Previous Message Guillaume Lelarge 2015-01-05 16:44:13 Re: Publish autovacuum informations