Subject: Re: [COMMITTERS] pgsql: Efficient transaction-controlled
	synchronous replication.
From: Simon Riggs <simon@2ndQuadrant.com>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Fujii Masao <masao.fujii@gmail.com>,
	PostgreSQL-development <pgsql-hackers@postgresql.org>
In-Reply-To: <AANLkTi=koHqna9WMm8_ATJN6c1GOLLp_0Tx6VswKhAdi@mail.gmail.com>
References: <E1PwMns-00047O-UU@gemulon.postgresql.org>
	<AANLkTikJmP+bo1N-mFUWEpJiV6_OKisYw512OGeTJUbm@mail.gmail.com>
	<AANLkTinHrymKd56m5AfawCdujuNM6B2g_--9UiOSSKGx@mail.gmail.com>
	<AANLkTinbJDsrkF9rsy8Wh_hrrrPP7CQaNcND_1A-aLMe@mail.gmail.com>
	<AANLkTi=f+EtkPU+rRgz=Ay-X80OvCGvxK_O6LpL9_dss@mail.gmail.com>
	<AANLkTi=koHqna9WMm8_ATJN6c1GOLLp_0Tx6VswKhAdi@mail.gmail.com>
Content-Type: text/plain; charset="UTF-8"
Date: Thu, 17 Mar 2011 17:56:36 +0000
Message-ID: <1300384596.18619.1823.camel@ebony>
Mime-Version: 1.0
Content-Transfer-Encoding: 7bit

On Thu, 2011-03-17 at 13:46 -0400, Robert Haas wrote:
> On Fri, Mar 11, 2011 at 5:46 AM, Fujii Masao <masao.fujii@gmail.com> wrote:
> > On Fri, Mar 11, 2011 at 5:04 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> >>>        if ((wrote_xlog && XactSyncCommit) || forceSyncCommit || nrels > 0 ||
> >>> SyncRepRequested())
> >>>
> >>> Whenever synchronous_replication is TRUE, we disable synchronous_commit.
> >>> But, before disabling that, we should check also max_wal_senders and
> >>> synchronous_standby_names? Otherwise, synchronous_commit can
> >>> be disabled unexpectedly even in non replication case.
> >>
> >> Yeah, that's bad.  At the risk of repeating myself, I don't think this
> >> code should be checking SyncRepRequested() in the first place.  If the
> >> user has turned off synchronous_commit, then we should just commit
> >> asynchronously, even if sync rep is otherwise in force.  Otherwise,
> >> this if statement is going to get really complicated.   The logic is
> >> already at least mildly wrong here anyway: clearly we do NOT need to
> >> commit synchronously if the transaction has not written xlog, even if
> >> sync rep is enabled.
> >
> > Yeah, not to wait for replication when synchronous_commit is disabled
> > seems to be more reasonable.
> 
> On further review, I've changed my mind.  Making synchronous_commit
> trump synchronous_replication is appealing conceptually, but it's
> going to lead to some weird corner cases.  For example, a transaction
> that drops a non-temporary relation always commits synchronously; and
> 2PC also ignores synchronous_commit.  In the case where
> synchronous_commit=off and synchronous_replication=on, we'd either
> have to decide that these sorts of transactions aren't going to
> replicate synchronously (which would give synchronous_commit a rather
> long reach into areas it doesn't currently touch) or else that it's OK
> for CREATE TABLE foo () to be totally asynchronous but that DROP TABLE
> foo requires sync commit AND sync rep.  That's pretty weird.
> 
> What makes more sense to me after having thought about this more
> carefully is to simply make a blanket rule that when
> synchronous_replication=on, synchronous_commit has no effect.  That is
> easy to understand and document.  I'm inclined to think it's OK to let
> synchronous_replication have this effect even if max_wal_senders=0 or
> synchronous_standby_names=''; you shouldn't turn
> synchronous_replication on just for kicks, and I don't think we want
> to complicate the test in RecordTransactionCommit() more than
> necessary.  We should, however, adjust the logic so that a transaction
> which has not written WAL can still commit asynchronously, because
> such a transaction has only touched temp or unlogged tables and so
> it's not important for it to make it to the standby, where that data
> doesn't exist anyway.

Agree to that.

Not read your other stuff yet, will do that later.

-- 
 Simon Riggs           http://www.2ndQuadrant.com/books/
 PostgreSQL Development, 24x7 Support, Training and Services