Re: pg_receivexlog add synchronous mode

On Tue, Jun 24, 2014 at 3:18 PM, <furuyao(at)pm(dot)nttdata(dot)co(dot)jp> wrote:
>> I found that this patch breaks --status-interval option of
>> pg_receivexlog when -m option which the patch introduced is supplied.
>> When -m is set, pg_receivexlog tries to send the feedback message as soon
>> as it flushes WAL file even if status interval timeout has not been passed
>> yet. If you want to send the feedback as soon as WAL is written or flushed,
>> like walreceiver does, you need to extend --status-interval option, for
>> example, so that it accepts the value "-1" which means enabling that
>> behavior.
>>
>> Including this change in your original patch would make it more difficult
>> to review. I think that you should implement this as separate patch.
>> Thought?
> As your comments, the current specification to ignore the --status-intarvall.
> It is necessary to respond immediately to synchronize.
>
> It is necessary to think about specifications the --status-intarvall.
> So I revised it to a patch of flushmode which performed flush by a timing same as walreceiver.

I'm not sure if it's good idea to call the feature which you'd like to
add as 'flush mode'.
ISTM that 'flush mode' is vague and confusion for users. Instead, what
about adding
something like --fsync-interval which pg_recvlogical supports?

> A changed part deletes the feedback message after flush, and transmitted the feedback message according to the status interval.
> Change to flushmode from syncmode the mode name, and fixed the document.

+ * Receive a message available from XLOG stream, blocking for
+ * maximum of 'timeout' ms.

The above comment seems incorrect because 'timeout' is boolean argument.

+ FD_ZERO(&input_mask);
+ FD_SET(PQsocket(conn), &input_mask);
+ if (standby_message_timeout)

Why did you get rid of the check of 'still_sending' flag here? Originally the
flag was checked but not in the patch.

+ r = rcv_receive(true , &copybuf, conn,
standby_message_timeout, last_status, now);

When the return value is -2 (i.e., an error happend), we should go to
the 'error' label.

ISTM that stream_stop() should be called every time a message is
processed. But the
patch changes pg_receivexlog so that it keeps processing the received
data without
calling stream_stop(). This seems incorrect.

'copybuf' needs to be free'd every time new message is received. But you seem to
have forgotten to do that when rcv_receive() with no timeout is called.

Regards,

--
Fujii Masao