On Thu, May 7, 2020 at 9:44 PM Robert Haas <robertmh...@gmail.com> wrote: > > On Wed, May 6, 2020 at 11:15 AM Robert Haas <robertmh...@gmail.com> wrote: > > Oh, good point. v2 attached. >
While looking at this, I noticed that caller (perform_base_backup) of do_pg_start_backup, sets the backup phase as PROGRESS_BASEBACKUP_PHASE_WAIT_CHECKPOINT whereas, in do_pg_start_backup, we do collect the information about all tablespaces after the checkpoint. I am not sure if it is long enough that we consider having a separate phase for it. Without your patch, it was covered under PROGRESS_BASEBACKUP_PHASE_ESTIMATE_BACKUP_SIZE phase which doesn't appear to be a bad idea. > Here's v3, with one more small cleanup. I noticed tblspc_map_file is > initialized to NULL and then unconditionally reset to the return value > of makeStringInfo(), and then later tested to see whether it is NULL. > It can't be, because makeStringInfo() doesn't return NULL. So the > attached version deletes the superfluous initialization and the > superfluous test. > This change looks fine to me. -- With Regards, Amit Kapila. EnterpriseDB: http://www.enterprisedb.com
