Re: Clean switchover

From: Andres Freund <andres(at)2ndquadrant(dot)com>
To: Stephen Frost <sfrost(at)snowman(dot)net>
Cc: Magnus Hagander <magnus(at)hagander(dot)net>, Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Clean switchover
Date: 2013-06-12 12:55:22
Message-ID: 20130612125522.GA19792@awork2.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On 2013-06-12 08:48:39 -0400, Stephen Frost wrote:
> * Magnus Hagander (magnus(at)hagander(dot)net) wrote:
> > On Wed, Jun 12, 2013 at 1:48 PM, Andres Freund <andres(at)2ndquadrant(dot)com> wrote:
> > > On 2013-06-12 07:53:29 +0900, Fujii Masao wrote:
> > >> The attached patch fixes this problem. It just changes walsender so that it
> > >> waits for all the outstanding WAL records to be replicated to the standby
> > >> before closing the replication connection.
> > >
> > > Imo this is a fix that needs to get backpatched... The code tried to do
> > > this but failed, I don't think it really gives grounds for valid *new*
> > > concerns.
> >
> > +1 (without having looked at the code itself, it's definitely a
> > behaviour that needs to be fixed)
>
> Yea, I was also thinking it would be reasonable to backpatch this; it
> really looks like a bug that we're allowing this to happen today.
>
> So, +1 on a backpatch for me. I've looked at the patch (it's a
> one-liner, plus some additional comments) but havn't looked through the
> overall code surrounding it.

I've read most of the surrounding code and I think the patch is as
sensible as it can be without reworking the whole walsender main loop
which seems like a job for another day.

I'd personally write
if (caughtup && !pq_is_send_pending() &&
sentPtr == MyWalSnd->flush)
as
if (caughtup && sentPtr == MyWalSnd->flush &&
!pq_is_send_pending())

Since pq_is_send_pending() basically can only be false if the flush
comparison is true. There's the tiny chance that we were sending a
message out just before which is why we should include the
!pq_is_send_pending() condition at all in that if().

But that's fairly, fairly minor.

Greetings,

Andres Freund

--
Andres Freund http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Robert Haas 2013-06-12 14:07:12 Re: Improvement of checkpoint IO scheduler for stable transaction responses
Previous Message Stephen Frost 2013-06-12 12:48:39 Re: Clean switchover