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

On Mon, Feb 3, 2020 at 1:17 PM Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> On 2020/02/02 14:59, Masahiko Sawada wrote:
> > On Fri, 31 Jan 2020 at 02:29, Fujii Masao <masao(dot)fujii(at)oss(dot)nttdata(dot)com> wrote:
> >> On 2020/01/30 12:58, Kyotaro Horiguchi wrote:
> >>> + WHEN 3 THEN 'stopping backup'::text
> >>>
> >>> I'm not sure, but the "stop" seems suggesting the backup is terminated
> >>> before completion. If it is following the name of the function
> >>> pg_stop_backup, I think the name is suggesting to stop "the state for
> >>> performing backup", not a backup.
> >>>
> >>> In the first place, the progress is about "backup" so it seems strange
> >>> that we have another phase after the "stopping backup" phase. It
> >>> might be better that it is "finishing file transfer" or such.
> >>>
> >>> "initializing"
> >>> -> "starting file transfer"
> >>> -> "transferring files"
> >>> -> "finishing file transfer"
> >>> -> "transaferring WAL"
> >>
> >> Better name is always welcome! If "stopping back" is confusing,
> >> what about "performing pg_stop_backup"? So
> >>
> >> initializing
> >> performing pg_start_backup
> >> streaming database files
> >> performing pg_stop_backup
> >> transfering WAL files
> >
> > Another idea I came up with is to show steps that take time instead of
> > pg_start_backup/pg_stop_backup, for better understanding the
> > situation. That is, "performing checkpoint" and "performing WAL
> > archive" etc, which engage the most of the time of these functions.
>
> Yeah, that's an idea. ISTM that "waiting for WAL archiving" sounds
> better than "performing WAL archive". Thought?
> I've not applied this change in the patch yet, but if there is no
> other idea, I'd like to adopt this.

If we are trying to "pg_stop_backup" in phase name, maybe we should
avoid "pg_start_backup"? Then maybe:

initializing
starting backup / waiting for [ backup start ] checkpoint to finish
transferring database files
finishing backup / waiting for WAL archiving to finish
transferring WAL files

?

Some comments on documentation changes in v2 patch:

+ Amount of data already streamed.

"already" may be redundant. For example, in pg_start_progress_vacuum,
heap_blks_scanned is described as "...blocks scanned", not "...blocks
already scanned".

+ <entry><structfield>tablespace_total</structfield></entry>
+ <entry><structfield>tablespace_streamed</structfield></entry>

Better to use plural tablespaces_total and tablespaces_streamed for consistency?

+ The WAL sender process is currently performing
+ <function>pg_start_backup</function> and setting up for
+ making a base backup.

How about "taking" instead of "making" in the above sentence?

- <varlistentry>
+ <varlistentry id="protocol-replication-base-backup" xreflabel="BASE_BACKUP">

I don't see any new text in the documentation patch that uses above
xref, so no need to define it?

Thanks,
Amit