Re: pg_receivexlog add synchronous mode

This patch is modified the comment.
Each comment is coping as follows.

> Could you update the status of this patch from "Waiting on Author" to
> "Needs Review" when you post the revised version of the patch?
Thank you for pointing out. 
From now on, I will update status when I post the patch.

> +        How often should <application>pg_receivexlog</application>
> issue sync
> +        commands to ensure the received WAL file is safely
> +        flushed to disk without being asked by the server to do so.
> 
> "without being asked by the server to do so" implies that the server asks
> pg_receivexlog to flush WAL files periodically?
I think that the sentence means the following.
Without waiting for the feedback request from the server, select is timed out and flush is checked. 

Because I cause misunderstanding, I delete a sentence.

>  Specifying
> +        an interval of <literal>0</literal> together the consecutive
> data.
> 
> This text looks corrupted. What does this mean?
> 
> +        Not specifying an interval disables issuing fsyncs altogether,
> +        while still reporting progress the server.
> 
> No. Even when the option is not specified, WAL files should be flushed
> at WAL file switch, like current pg_receivexlog does. If you want to
> disable the flush completely, you can change the option so that it accepts
> -1 which means to disable the flush, for example.
Fix to description "issuing fsyncs at only  WAL file close". 

> +    printf(_("  -F  --fsync-interval=SECS\n"
> +             "                         frequency of syncs to the
> output file (default: file close only)\n"));
> 
> It's better to use "transaction log files" rather than "output file"
> here.
Fix as you pointed out.

> SECS should be INTERVAL for the sake of consistency with
> --stat-interval's help message?
Fix as you pointed out.

> + * fsync_interval controls how often we flush to the received
> + * WAL file, in seconds.
> 
> "seconds" should be "miliseconds"?
Fix as you pointed out.

> The patch changes pg_receivexlog so that it keep processing the
> continuous messages without calling stream_stop(). But as I commented
> before, stream_stop should be called every time the message is received?
> Otherwise pg_basebackup background WAL receiver might not be able to stop
> streaming at proper point.
FIx the calling stream_stop() with 1 message processing is complete. 

> The flush interval is checked and WAL files are flushed only when
> CopyStreamReceive() returns 0, i.e., there is no message available and
> the timeout occurs. Why did you do that? I'm afraid that
> CopyStreamReceive() always waits for at least one second before WAL files
> are flushed even when --fsync-interval is set to 0.
CopyStreamReceive() is according to pg_recvlogical --fsync-interval and 
--status-interval shorter intervals runs the wait.
About specifying an interval of zero.
Every flush to continuously message,  so no problem will wait one second.

> Why don't you update output_last_status when WAL file is flushed and
> closed?
About the closed, add the update step.
About the flush, according to pg_recvlogical, update is performed after an interval check before flush. 
Therefore not modify.

Regards,

--
Furuya Osamu