From: | Robert Haas <robertmhaas(at)gmail(dot)com> |
---|---|
To: | Fujii Masao <masao(dot)fujii(at)gmail(dot)com> |
Cc: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com>, pgsql-hackers(at)postgresql(dot)org |
Subject: | Re: Sync Rep for 2011CF1 |
Date: | 2011-02-15 17:06:38 |
Message-ID: | AANLkTinn9OuD8xwt+6zdyqYvqj9J1hwjPzKq=-ekqndk@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Mon, Feb 14, 2011 at 12:08 AM, Fujii Masao <masao(dot)fujii(at)gmail(dot)com> wrote:
> On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas
> <heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
>> I committed the patch with those changes, and some minor comment tweaks and
>> other kibitzing.
>
> + * 'd' means a standby reply wrapped in a COPY BOTH packet.
> + */
>
> Typo: s/COPY BOTH/CopyData
Fixed.
> + msgtype = pq_getmsgbyte(&input_message);
> + if (msgtype != 'r')
> + ereport(COMMERROR,
> + (errcode(ERRCODE_PROTOCOL_VIOLATION),
> + errmsg("unexpected message type %c", msgtype)));
>
> I think that proc_exit(0) needs to be called in error case.
Fixed.
> + static StringInfoData input_message;
> + StandbyReplyMessage reply;
> + char msgtype;
> +
> + initStringInfo(&input_message);
>
> Doesn't the repeat of initStringInfo() cause the memory leak?
Yeah. Fixed, I hope.
> @@ -518,6 +584,7 @@ WalSndLoop(void)
> {
> if (!XLogSend(output_message, &caughtup))
> break;
> + ProcessRepliesIfAny();
>
> Why is ProcessRepliesIfAny() required there?
I'm not sure if that's 100% necessary, but it seems harmless enough.
> We added new columns "write_location", "flush_location" and
> "apply_location". So, for the sake of consistency, the column
> name "sent_location" should be changed to "send_location"?
I was thinking about stream_location or streaming_location, per
discussion on the other thread.
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company
From | Date | Subject | |
---|---|---|---|
Next Message | Robert Haas | 2011-02-15 17:06:53 | Re: Sync Rep for 2011CF1 |
Previous Message | Tom Lane | 2011-02-15 16:59:54 | Re: Fix for Index Advisor related hooks |