On Fri, 31 Jan 2020 at 02:29, Fujii Masao <masao.fu...@oss.nttdata.com> wrote: > > > > On 2020/01/30 12:58, Kyotaro Horiguchi wrote: > > At Wed, 29 Jan 2020 23:16:08 +0900, Fujii Masao > > <masao.fu...@oss.nttdata.com> wrote in > >> Hi, > >> > >> pg_basebackup reports the backup progress if --progress option is > >> specified, > >> and we can monitor it in the client side. I think that it's useful if > >> we can > >> monitor the progress information also in the server side because, for > >> example, > >> we can easily check that by using SQL. So I'd like to propose > >> pg_stat_progress_basebackup view that allows us to monitor the > >> progress > >> of pg_basebackup, in the server side. Thought? > >> > >> POC patch is attached. > > > > | postgres=# \d pg_stat_progress_basebackup > > | View "pg_catalog.pg_stat_progress_basebackup" > > | Column | Type | Collation | Nullable | Default > > | ---------------------+---------+-----------+----------+--------- > > | pid | integer | | | > > | phase | text | | | > > | backup_total | bigint | | | > > | backup_streamed | bigint | | | > > | tablespace_total | bigint | | | > > | tablespace_streamed | bigint | | | > > > > I think the view needs client identity such like host/port pair > > besides pid (host/port pair fails identify client in the case of > > unix-sockets.). > > I don't think this is job of a progress reporting. If those information > is necessary, we can join this view with pg_stat_replication using > pid column as the join key. > > > Also elapsed time from session start might be > > useful. > > +1 > I think that not only pg_stat_progress_basebackup but also all the other > progress views should report the time when the target command was > started and the time when the phase was last changed. Those times > would be useful to estimate the remaining execution time from the > progress infromation. > > > pg_stat_progress_acuum has datid, datname and relid. > > > > + if (backup_total > 0 && backup_streamed > backup_total) > > + { > > + backup_total = backup_streamed; > > > > Do we need the condition "backup_total > 0"? > > I added the condition for the case where --progress option is not supplied > in pg_basebackup, i.e., the case where the total amount of backup is not > estimated at the beginning of the backup. In this case, total_backup is > always 0. > > > + int tblspc_streamed = 0; > > + > > + pgstat_progress_update_param(PROGRESS_BASEBACKUP_PHASE, > > + > > PROGRESS_BASEBACKUP_PHASE_STREAM_BACKUP); > > > > This starts "streaming backup" phase with backup_total = 0. Coudln't > > we move to that phase after setting backup total and tablespace total? > > That is, just after calling SendBackupHeader(). > > OK, that's a bit less confusing for users. I will change in that way. > > > + 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. Regards, -- Masahiko Sawada http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services