On Fri, Mar 11, 2011 at 5:46 AM, Fujii Masao <masao.fu...@gmail.com> wrote: > On Fri, Mar 11, 2011 at 5:04 AM, Robert Haas <robertmh...@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. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
