Re: libpq changes for synchronous replication

From: Greg Smith <greg(at)2ndquadrant(dot)com>
To: Alvaro Herrera <alvherre(at)commandprompt(dot)com>
Cc: Fujii Masao <masao(dot)fujii(at)gmail(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Robert Haas <robertmhaas(at)gmail(dot)com>, 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-05 18:07:33
Message-ID: 4CFBD4E5.50504@2ndquadrant.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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.

Alvaro Herrera wrote:
> Excerpts from Fujii Masao's message of jue nov 25 10:47:12 -0300 2010:
>
>
>> The attached patch s/CopyXLog/CopyBoth/g and adds the description
>> about CopyBoth into the COPY section.
>>
>
> I gave this a look. It seems good, but I'm not sure about this bit:
>
> + case 'W': /* Start Copy Both */
> + /*
> + * We don't need to use getCopyStart here since CopyBothResponse
> + * specifies neither the copy format nor the number of columns in
> + * the Copy data. They should be always zero.
> + */
> + conn->result = PQmakeEmptyPGresult(conn, PGRES_COPY_OUT);
> + if (!conn->result)
> + return;
> + conn->asyncStatus = PGASYNC_COPY_BOTH;
> + conn->copy_already_done = 0;
> + break;
>
> 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?
>
> (The paragraph added to the docs is also a bit too specific about this
> being used exclusively in streaming replication, ISTM)
>
>
>> 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.
>

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Kevin Grittner 2010-12-05 18:11:08 Re: serializable read only deferrable
Previous Message Greg Smith 2010-12-05 17:55:39 Re: WIP patch for parallel pg_dump