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);

Reply via email to