On Thu, May 7, 2020 at 9:44 PM Robert Haas <robertmhaas@gmail.com> wrote:
>
> On Wed, May 6, 2020 at 11:15 AM Robert Haas <robertmhaas@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