On Thu, 2011-03-17 at 13:46 -0400, Robert Haas wrote: > 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.
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 -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
