Re: libpq changes for synchronous replication

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>
Cc: Greg Smith <greg(at)2ndquadrant(dot)com>, Alvaro Herrera <alvherre(at)commandprompt(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: libpq changes for synchronous replication
Date: 2010-12-11 14:37:06
Message-ID: AANLkTi=FDe_Q_RC9=jenAGA3+b_0G1VXsjN0fL6J=bH4@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Mon, Dec 6, 2010 at 12:54 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Mon, Dec 6, 2010 at 3:07 AM, Greg Smith <greg(at)2ndquadrant(dot)com> wrote:
>> The one time this year top-posting seems appropriate...this patch seems
>> stalled waiting for some sort of response to the concerns Alvaro raised
>> here.
>
> Sorry for the delay. I didn't have the time.
>
>> I gave this a look.  It seems good, but I'm not sure about this bit:
>
> Thanks for the review!
>
>> I guess this was OK when this was conceived as CopyXlog, but since it's
>> now a generic mechanism, this seems a bit unwise.  Should this be
>> reconsidered so that it's possible to change the format or number of
>> columns?
>
> I changed CopyBothResponse message so that it includes the format
> and number of columns of copy data. Please see the attached patch.
>
>> (The paragraph added to the docs is also a bit too specific about this
>> being used exclusively in streaming replication, ISTM)
>
> Yes. But it seems difficult to generalize the docs more because currently
> only SR uses Copy-both. So I had to write that, for example, the condition
> to get into the state is only "START REPLICATION" command.
>
>> While modifying the code, it occurred to me that we might have to add new
>> ExecStatusType like PGRES_COPY_BOTH and use that for CopyBoth mode,
>> for the sake of consistency. But since it's just alias of PGRES_COPY_BOTH
>> for now, i.e., there is no specific behavior for that ExecStatusType, I
>> don't
>> think that it's worth adding that yet.
>>
>>
>> I'm not so sure about this.  If we think that it's worth adding a new
>> possible state, we should do so now; we will not be able to change this
>> behavior later.
>
> OK. I added that new state.

Committed with just a few changes to the documentation.

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Pavel Stehule 2010-12-11 15:03:42 Re: proposal: auxiliary functions for record type
Previous Message Simon Riggs 2010-12-11 14:28:22 Re: PS display and standby query conflict