Re: Sync Rep for 2011CF1

On Fri, Feb 11, 2011 at 4:06 AM, Heikki Linnakangas
<heikki.linnakangas@enterprisedb.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