On Mon, Feb 3, 2020 at 1:17 PM Fujii Masao <masao.fujii@oss.nttdata.com> wrote:
> On 2020/02/02 14:59, Masahiko Sawada wrote:
> > On Fri, 31 Jan 2020 at 02:29, Fujii Masao <masao.fujii@oss.nttdata.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
