Re: pg_receivexlog add synchronous mode

On Fri, Jul 4, 2014 at 7:45 PM, <furuyao(at)pm(dot)nttdata(dot)co(dot)jp> wrote:
>> > Thanks for reviewing the patch!
>> >
>> > I think that this refactoring patch is useful for improving source
>> > code readability and making the future patches simpler, whether we
>> > adopt your patch or not. So, barring any objections, I'm thinking to
>> > commit this refactoring patch.
>>
>> Committed!
> It is a patch that added the --fsync-interval option.
> Interface of --fsync-interval option was referring to the "pg_recvlogical.c".
>
> It is not judgement the flush on a per-message basis.
> It is judgment at the time of receipt stop of the message.
>
> If you specify a zero --fsync-interval make the flush at the same timing as the walreceiver .
> If you do not specify --fsync-interval, you will flush only when switching as WAL files as in the past.

Thanks for revising the patch!

Could you update the status of this patch from "Waiting on Author" to
"Needs Review" when you post the revised version of 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?

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.

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

SECS should be INTERVAL for the sake of consistency with --stat-interval's
help message?

+ * fsync_interval controls how often we flush to the received
+ * WAL file, in seconds.

"seconds" should be "miliseconds"?

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.

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.

Why don't you update output_last_status when WAL file is flushed
and closed?

Regards,

--
Fujii Masao