Re: pg_stat_progress_basebackup - progress reporting for pg_basebackup, in the server side

On 2020/02/18 21:31, Fujii Masao wrote:
>
>
> On 2020/02/18 16:53, Amit Langote wrote:
>> On Tue, Feb 18, 2020 at 4:42 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
>>> On 2020/02/18 16:02, Amit Langote wrote:
>>>> I noticed that there is missing </para> tag in the documentation changes:
>>>
>>> Could you tell me where I should add </para> tag?
>>>
>>>> +     <row>
>>>> +      <entry><literal>waiting for checkpoint to finish</literal></entry>
>>>> +      <entry>
>>>> +       The WAL sender process is currently performing
>>>> +       <function>pg_start_backup</function> to set up for
>>>> +       taking a base backup, and waiting for backup start
>>>> +       checkpoint to finish.
>>>> +      </entry>
>>>> +     <row>
>>>>
>>>> There should be a </row> between </entry> and <row> at the end of the
>>>> hunk shown above.
>>>
>>> Will fix. Thanks!
>>
>> Just to clarify, that's the missing </para> tag I am talking about above.
>
> OK, so I added </row> tag just after the above </entry>.
>
>>>> +  <para>
>>>> +   Whenever <application>pg_basebackup</application> is taking a base
>>>> +   backup, the <structname>pg_stat_progress_basebackup</structname>
>>>> +   view will contain a row for each WAL sender process that is currently
>>>> +   running <command>BASE_BACKUP</command> replication command
>>>> +   and streaming the backup.
>>>>
>>>> I understand that you wrote "Whenever pg_basebackup is taking a
>>>> backup...", because description of other views contains a similar
>>>> starting line.  But, it may not only be pg_basebackup that would be
>>>> served by this view, no?  It could be any tool that speaks Postgres'
>>>> replication protocol and thus be able to send a BASE_BACKUP command.
>>>> If that is correct, I would write something like "When an application
>>>> is taking a backup" or some such without specific reference to
>>>> pg_basebackup.  Thoughts?
>>>
>>> Yeah, there may be some such applications. But most users would
>>> use pg_basebackup, so getting rid of the reference to pg_basebackup
>>> would make the description a bit difficult-to-read. Also I can imagine
>>> that an user of those backup applications would get to know
>>> the progress reporting view from their documents. So I prefer
>>> the existing one or something like "Whenever an application like
>>>    pg_basebackup ...". Thought?
>>
>> Sure, "an application like pg_basebackup" sounds fine to me.
>
> OK, I changed the doc that way. Attached the updated version of the patch.

Attached is the updated version of the patch.

The previous patch used only pgstat_progress_update_param()
even when updating multiple values. Since those updates are
not atomic, this can cause readers of the values to see
the intermediate states. To avoid this issue, the latest patch
uses pgstat_progress_update_multi_param(), instead.

Regards,

--
Fujii Masao
NTT DATA CORPORATION
Advanced Platform Technology Group
Research and Development Headquarters