Horton
Menu

Re: Stefan's bug (was: max_standby_delay considered harmful)

Robert Haas <robertmhaas@gmail.com>

View thread
From: Robert Haas <robertmhaas@gmail.com>
To: Simon Riggs <simon@2ndquadrant.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Stefan Kaltenbrunner <stefan@kaltenbrunner.cc>, Fujii Masao <masao.fujii@gmail.com>, Andres Freund <andres@anarazel.de>, Florian Pflug <fgp@phlo.org>, Dimitri Fontaine <dfontaine@hi-media.com>, Tom Lane <tgl@sss.pgh.pa.us>, pgsql-hackers <pgsql-hackers@postgresql.org>, Bruce Momjian <bruce@momjian.us>, Greg Smith <greg@2ndquadrant.com>, Josh Berkus <josh@agliodbs.com>
Date: 2010-05-17T10:55:24Z
Lists: pgsql-hackers 
On Mon, May 17, 2010 at 6:41 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
> On Mon, 2010-05-17 at 06:30 -0400, Robert Haas wrote:
>> On Mon, May 17, 2010 at 3:13 AM, Simon Riggs <simon@2ndquadrant.com> wrote:
>> > On Sun, 2010-05-16 at 21:25 -0400, Robert Haas wrote:
>> >
>> >> I have what I believe is
>> >> an equivalent but simpler implementation, which is attached.
>> >
>> > There's no code comments to explain this, so without in-depth analysis
>> > of the problem, Masao's patch and this one its not possible to say
>> > anything.
>> >
>> > Please explain in detail why its the right approach and put that in a
>> > comment, so we'll understand now and in the future.
>>
>> The explanation is what I wrote in my previous email: a smart shutdown
>> request during recovery should be treated the same way BEFORE the
>> postmaster has been asked to start the background writer and AFTER the
>> postmaster has been asked to start the background writer.  I'll think
>> up a suitable comment.
>
> I think we should review Masao's patch and ask him to make any changes
> we think are appropriate. There's no benefit to have multiple patch
> authors at one time.

I did review his patch.  It duplicates a few lines of logic and I
found a way to avoid that, so I proposed it.  That seems totally
normal to me and I'm not sure what you're concerned about.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise Postgres Company