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

On Mon, Apr 4, 2011 at 4:54 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Mar 25, 2011 at 8:53 PM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
>> On Sat, Mar 19, 2011 at 12:07 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>> On Fri, Mar 18, 2011 at 10:55 AM, Greg Stark <gsstark(at)mit(dot)edu> wrote:
>>>> On Thu, Mar 17, 2011 at 5:46 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
>>>>> 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.
>>>>
>>>> For what it's worth "has no effect" doesn't make much sense to me.
>>>> It's a boolean, either commits are going to block or they're not.
>>>>
>>>> What happened to the idea of a three-way switch?
>>>>
>>>> synchronous_commit = off
>>>> synchronous_commit = disk
>>>> synchronous_commit = replica
>>>>
>>>> With "on" being a synonym for "disk" for backwards compatibility.
>>>>
>>>> Then we could add more options later for more complex conditions like
>>>> waiting for one server in each data centre or waiting for one of a
>>>> certain set of servers ignoring the less reliable mirrors, etc.
>>>
>>> This is similar to what I suggested upthread, except that I suggested
>>> on/local/off, with the default being on.  That way if you set
>>> synchronous_standby_names, you get synchronous replication without
>>> changing another setting, but you can say local instead if for some
>>> reason you want the middle behavior.  If we're going to do it all with
>>> one GUC, I think that way makes more sense.  If you're running sync
>>> rep, you might still have some transactions that you don't care about,
>>> but that's what async commit is for.  It's a funny kind of transaction
>>> that we're OK with losing if we have a failover but we're not OK with
>>> losing if we have a local crash from which we recover without failing
>>> over.
>>
>> I'm OK with this.
>
> The attached patch merges synchronous_replication into synchronous_commit.
> With the patch, valid values of synchronous_commit are "on" (waits for local
> flush and sync rep), "off" (waits for neither local flush nor sync
> rep), and "local"
> (waits for only local flush).

Committed with some additional hacking. In particular, I believe that
your version made SYNCHRONOUS_COMMIT_LOCAL equivalent to
SYNCHRONOUS_COMMIT_OFF, which was wrong; and your replacement of
synchronous_replication by synchronous_commit in the docs was a bit
too formulaic; in particular, the section on setting up a basic sync
rep configuration said that all you needed to do was set
synchronous_commit=on, which clearly made no sense, since that was
neither necessary (since that's the default) nor sufficient (since you
have to set synchronous_standby_names).

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