Обсуждение: Primary not sending to synchronous standby

Hi,

On 2015-02-23 15:25:57 +0000, Thom Brown wrote:
> I've noticed that if the primary is started and then a base backup is
> immediately taken from it and started as as a synchronous standby, it
> doesn't replicate and the primary hangs indefinitely when trying to run any
> WAL-generating statements.  It only recovers when either the primary is
> restarted (which has to use a fast shutdown otherwise it also hangs
> forever), or the standby is restarted.
> 
> Here's a way of reproducing it:
> ...
> Note that if you run the commands one by one, there isn't a problem.  If
> you run it as a script, the standby doesn't connect to the primary.  There
> aren't any errors reported by either the standby or the primary.  The
> primary's wal sender process reports the following:
> 
> wal sender process rep_user 127.0.0.1(45243) startup waiting for 0/3000158
> 
> Anyone know why this would be happening?  And if this could be a problem in
> other scenarios?

Given that normally a walsender doesn't wait for syncrep I guess this is
the above backend just did authentication. If you gdb into the
walsender, what's the backtrace?

We previously had discussions about that being rather annoying; I
unfortunately don't remember enough of the thread to reference it
here. If it really is this, I think we should add some more smarts about
only enabling syncrep once a backend is fully up and maybe even remove
it from more scenarios during commits generally (e.g. if no xid was
assigned and we just pruned something).

Greetings,

Andres Freund

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

On 2015-02-23 16:38:44 +0100, Andres Freund wrote:
> I unfortunately don't remember enough of the thread to reference it
> here.

Found the right keywords. The threads below
http://archives.postgresql.org/message-id/369698E947874884A77849D8FE3680C2%40maumau
and
http://www.postgresql.org/message-id/5CF4ABBA67674088B3941894E22A0D25@maumau

Greetings,

Andres Freund

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

On 2015-02-23 15:48:25 +0000, Thom Brown wrote:
> On 23 February 2015 at 15:42, Andres Freund <andres@2ndquadrant.com> wrote:
>
> > On 2015-02-23 16:38:44 +0100, Andres Freund wrote:
> > > I unfortunately don't remember enough of the thread to reference it
> > > here.
> >
> > Found the right keywords. The threads below
> >
> > http://archives.postgresql.org/message-id/369698E947874884A77849D8FE3680C2%40maumau
> > and
> >
> > http://www.postgresql.org/message-id/5CF4ABBA67674088B3941894E22A0D25@maumau
> >
>
> Yes, this seems to be virtually the same issue reported.  The trace looks
> the same except for RecordTransactionCommit.

So, I proposed in
http://www.postgresql.org/message-id/20140707155113.GB1136@alap3.anarazel.de
that we make sequences assign a xid and only wait for syncrep when a xid
is assigned. The biggest blocker was that somebody would have to do some
code reviewing to find other locations that might need similar
treatment.

I did a, quick, grep for XLogInsert() and I think we're otherwise
fine. There's some debatable cases:

* XLOG_STANDBY_LOCK  doesn't force a xid to be assigned. I think it's
  harmless though, as we really only need to wait for that to be
  replicated if the transaction did something relevant (i.e. catalog
  changes). And those will force xid assignment.
* 2pc records don't assign a xid. But twophase.c does it's own waiting,
  so that's fine.
* Plain vacuums will not trigger waits. But I think that's good. There's
  really no need to wait if all that's been done is some cleanup without
  visible consequences.
* Fujii brought up that we might want to wait for XLOG_SWITCH - I don't
  really see why.
* XLOG_RESTORE_POINT is a similar candidate - I don't see really valid
  arguments for making 2pc wait.


The attached, untested, patch changes things so that we
a) only wait for syncrep if we both wrote WAL and had a xid assigned
b) use an async commit if we just had a xid assigned, without having
   written WAL, even if synchronous_commit = off
c) acquire a xid when WAL logging sequence changes (arguable at least
   one of the xid assignments is redundant, but it doesn't cost
   anything, so ...)

I think it makes sense to change a) and b) that way because there's no
need to wait for WAL flushes/syncrep waits when all that happened is
manipulations of temporary/unlogged tables or HOT pruning. It's slightly
wierd that the on-disk flush and the syncrep wait essentially used two
different mechanisms for deciding when to flush.

Comments? This is obviously just a POC, but I think something like this
does make a great deal of sense.

Thom, does that help?

Greetings,

Andres Freund

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

On 2015-02-23 17:53:59 +0100, Andres Freund wrote:
> On 2015-02-23 15:48:25 +0000, Thom Brown wrote:
> > On 23 February 2015 at 15:42, Andres Freund <andres@2ndquadrant.com> wrote:
> > 
> > > On 2015-02-23 16:38:44 +0100, Andres Freund wrote:
> > > > I unfortunately don't remember enough of the thread to reference it
> > > > here.
> > >
> > > Found the right keywords. The threads below
> > >
> > > http://archives.postgresql.org/message-id/369698E947874884A77849D8FE3680C2%40maumau
> > > and
> > >
> > > http://www.postgresql.org/message-id/5CF4ABBA67674088B3941894E22A0D25@maumau
> > >
> > 
> > Yes, this seems to be virtually the same issue reported.  The trace looks
> > the same except for RecordTransactionCommit.
> 
> So, I proposed in
> http://www.postgresql.org/message-id/20140707155113.GB1136@alap3.anarazel.de
> that we make sequences assign a xid and only wait for syncrep when a xid
> is assigned. The biggest blocker was that somebody would have to do some
> code reviewing to find other locations that might need similar
> treatment.
> 
> I did a, quick, grep for XLogInsert() and I think we're otherwise
> fine. There's some debatable cases:
> 
> * XLOG_STANDBY_LOCK  doesn't force a xid to be assigned. I think it's
>   harmless though, as we really only need to wait for that to be
>   replicated if the transaction did something relevant (i.e. catalog
>   changes). And those will force xid assignment.
> * 2pc records don't assign a xid. But twophase.c does it's own waiting,
>   so that's fine.
> * Plain vacuums will not trigger waits. But I think that's good. There's
>   really no need to wait if all that's been done is some cleanup without
>   visible consequences.
> * Fujii brought up that we might want to wait for XLOG_SWITCH - I don't
>   really see why.
> * XLOG_RESTORE_POINT is a similar candidate - I don't see really valid
>   arguments for making 2pc wait.
> 
> 
> The attached, untested, patch changes things so that we
> a) only wait for syncrep if we both wrote WAL and had a xid assigned
> b) use an async commit if we just had a xid assigned, without having
>    written WAL, even if synchronous_commit = off
> c) acquire a xid when WAL logging sequence changes (arguable at least
>    one of the xid assignments is redundant, but it doesn't cost
>    anything, so ...)
> 
> I think it makes sense to change a) and b) that way because there's no
> need to wait for WAL flushes/syncrep waits when all that happened is
> manipulations of temporary/unlogged tables or HOT pruning. It's slightly
> wierd that the on-disk flush and the syncrep wait essentially used two
> different mechanisms for deciding when to flush.

I think this patch is a clear improvement - and unless somebody protests
I'm planning to push it sometime not too far away.

What I'm wondering about is what we want to do with the back branches:
It's really rather ugly that enabling syncrep can prevent you from
logging in; especially if the connection that's blocked is the syncrep
walsender. It'll resolve itself if you kill the connections often
enough, but still, that's something you don't wan to have to do. The
patch isn't particularly complex and should only make things efficient,
so it'd be defensible to backpatch it. Thoughts?

Greetings,

Andres Freund

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

On 2015-02-23 17:09:24 +0000, Thom Brown wrote:
> On 23 February 2015 at 16:53, Andres Freund <andres@2ndquadrant.com> wrote:
> > Comments? This is obviously just a POC, but I think something like this
> > does make a great deal of sense.
> >
> > Thom, does that help?

> Yeah, this appears to eliminate the problem, at least in the case I
> reported.

I've pushed a somewhat more evolved version of this after more testing.

Greetings,

Andres Freund

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