Re: Sync Rep for 2011CF1

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

+ 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.

+ static StringInfoData input_message;
+ StandbyReplyMessage reply;
+ char msgtype;
+
+ initStringInfo(&input_message);

Doesn't the repeat of initStringInfo() cause the memory leak?

@@ -518,6 +584,7 @@ WalSndLoop(void)
{
if (!XLogSend(output_message, &caughtup))
break;
+ ProcessRepliesIfAny();

Why is ProcessRepliesIfAny() required there?

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"?

Regards,.

--
Fujii Masao
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center