On Tue, Feb 4, 2020 at 10:34 AM Amit Langote <amitlangot...@gmail.com> wrote: > Reading this: > > + <entry><structfield>backup_total</structfield></entry> > + <entry><type>bigint</type></entry> > + <entry> > + Total amount of data that will be streamed. If progress reporting > + is not enabled in <application>pg_basebackup</application> > + (i.e., <literal>--progress</literal> option is not specified), > + this is <literal>0</literal>. > > I wonder how hard would it be to change basebackup.c to always set > backup_total, irrespective of whether --progress is specified with > pg_basebackup or not? It seems quite misleading to leave it set to 0, > because one may panic that they have lost their data, that is, if they > haven't first read the documentation.
For example, the attached patch changes basebackup.c to always set tablespaceinfo.size, irrespective of whether --progress was passed with BASE_BACKUP command. It passes make check-world, so maybe safe. Maybe it would be a good idea to add a couple of more comments around tablespaceinfo struct definition, such as how 'size' is to be interpreted. Thanks, Amit
diff --git a/src/backend/access/transam/xlog.c b/src/backend/access/transam/xlog.c index 3813eadfb4..b4eb7063b8 100644 --- a/src/backend/access/transam/xlog.c +++ b/src/backend/access/transam/xlog.c @@ -10227,8 +10227,7 @@ issue_xlog_fsync(int fd, XLogSegNo segno) XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, StringInfo labelfile, List **tablespaces, - StringInfo tblspcmapfile, bool infotbssize, - bool needtblspcmapfile) + StringInfo tblspcmapfile, bool needtblspcmapfile) { bool exclusive = (labelfile == NULL); bool backup_started_in_recovery = false; @@ -10512,7 +10511,7 @@ do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, ti->oid = pstrdup(de->d_name); ti->path = pstrdup(buflinkpath.data); ti->rpath = relpath ? pstrdup(relpath) : NULL; - ti->size = infotbssize ? sendTablespace(fullpath, true) : -1; + ti->size = sendTablespace(fullpath, true); if (tablespaces) *tablespaces = lappend(*tablespaces, ti); diff --git a/src/backend/access/transam/xlogfuncs.c b/src/backend/access/transam/xlogfuncs.c index 20316539b6..c0b46dceae 100644 --- a/src/backend/access/transam/xlogfuncs.c +++ b/src/backend/access/transam/xlogfuncs.c @@ -76,7 +76,7 @@ pg_start_backup(PG_FUNCTION_ARGS) if (exclusive) { startpoint = do_pg_start_backup(backupidstr, fast, NULL, NULL, - NULL, NULL, false, true); + NULL, NULL, true); } else { @@ -94,7 +94,7 @@ pg_start_backup(PG_FUNCTION_ARGS) register_persistent_abort_backup_handler(); startpoint = do_pg_start_backup(backupidstr, fast, NULL, label_file, - NULL, tblspc_map_file, false, true); + NULL, tblspc_map_file, true); } PG_RETURN_LSN(startpoint); diff --git a/src/backend/replication/basebackup.c b/src/backend/replication/basebackup.c index dea8aab45e..f92695ce91 100644 --- a/src/backend/replication/basebackup.c +++ b/src/backend/replication/basebackup.c @@ -243,8 +243,7 @@ perform_base_backup(basebackup_options *opt) startptr = do_pg_start_backup(opt->label, opt->fastcheckpoint, &starttli, labelfile, &tablespaces, - tblspc_map_file, - opt->progress, opt->sendtblspcmapfile); + tblspc_map_file, opt->sendtblspcmapfile); /* * Once do_pg_start_backup has been called, ensure that any failure causes @@ -274,7 +273,7 @@ perform_base_backup(basebackup_options *opt) /* Add a node for the base directory at the end */ ti = palloc0(sizeof(tablespaceinfo)); - ti->size = opt->progress ? sendDir(".", 1, true, tablespaces, true) : -1; + ti->size = sendDir(".", 1, true, tablespaces, true); tablespaces = lappend(tablespaces, ti); /* Send tablespace header */ diff --git a/src/include/access/xlog.h b/src/include/access/xlog.h index 98b033fc20..d4ca68900a 100644 --- a/src/include/access/xlog.h +++ b/src/include/access/xlog.h @@ -345,7 +345,7 @@ typedef enum SessionBackupState extern XLogRecPtr do_pg_start_backup(const char *backupidstr, bool fast, TimeLineID *starttli_p, StringInfo labelfile, - List **tablespaces, StringInfo tblspcmapfile, bool infotbssize, + List **tablespaces, StringInfo tblspcmapfile, bool needtblspcmapfile); extern XLogRecPtr do_pg_stop_backup(char *labelfile, bool waitforarchive, TimeLineID *stoptli_p);