Re: [COMMITTERS] pgsql: Efficient transaction-controlled synchronous replication.

On Fri, Mar 11, 2011 at 5:46 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Mar 11, 2011 at 5:04 AM, Robert Haas <robertmhaas(at)gmail(dot)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.

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