Re: tablespace_map code cleanup

2020-06-17 Thread Robert Haas
On Wed, May 27, 2020 at 8:11 PM Kyotaro Horiguchi wrote: > That makes sense to me. Thanks! Great. Thanks to you and Amit for reviewing. I have committed the patches. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company

Re: tablespace_map code cleanup

2020-05-27 Thread Kyotaro Horiguchi
At Wed, 27 May 2020 07:57:38 -0400, Robert Haas wrote in > On Wed, May 13, 2020 at 10:43 PM Kyotaro Horiguchi > wrote: > > About 0002, > > > > + boolsendtblspclinks = true; > > > > The boolean seems to me useless since it is always the inverse of > > opt->sendt

Re: tablespace_map code cleanup

2020-05-27 Thread Robert Haas
On Wed, May 13, 2020 at 10:43 PM Kyotaro Horiguchi wrote: > About 0002, > > + boolsendtblspclinks = true; > > The boolean seems to me useless since it is always the inverse of > opt->sendtblspcmapfile when it is used. Well, I think it might have some documentatio

Re: tablespace_map code cleanup

2020-05-13 Thread Kyotaro Horiguchi
At Wed, 13 May 2020 15:10:30 -0400, Robert Haas wrote in > On Tue, May 12, 2020 at 10:26 PM Amit Kapila wrote: > > I was trying to say that tablespace listing will happen under > > PROGRESS_BASEBACKUP_PHASE_WAIT_CHECKPOINT phase which could be a > > problem if it is a costly operation but as yo

Re: tablespace_map code cleanup

2020-05-13 Thread Robert Haas
On Tue, May 12, 2020 at 10:26 PM Amit Kapila wrote: > I was trying to say that tablespace listing will happen under > PROGRESS_BASEBACKUP_PHASE_WAIT_CHECKPOINT phase which could be a > problem if it is a costly operation but as you said it is pretty cheap > so I think we don't need to bother about

Re: tablespace_map code cleanup

2020-05-12 Thread Amit Kapila
On Wed, May 13, 2020 at 1:54 AM Robert Haas wrote: > > On Tue, May 12, 2020 at 2:23 AM Amit Kapila wrote: > > 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_

Re: tablespace_map code cleanup

2020-05-12 Thread Robert Haas
On Tue, May 12, 2020 at 2:23 AM Amit Kapila wrote: > 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 > tablespa

Re: tablespace_map code cleanup

2020-05-11 Thread Amit Kapila
On Thu, May 7, 2020 at 9:44 PM Robert Haas wrote: > > On Wed, May 6, 2020 at 11:15 AM Robert Haas 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

Re: tablespace_map code cleanup

2020-05-07 Thread Robert Haas
On Wed, May 6, 2020 at 11:15 AM Robert Haas wrote: > Oh, good point. v2 attached. 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 ca

Re: tablespace_map code cleanup

2020-05-06 Thread Robert Haas
On Mon, May 4, 2020 at 5:24 AM Amit Kapila wrote: > If we want to move the calculation of size for tablespaces in the > caller then I think we also need to do something about the progress > reporting related to phase > PROGRESS_BASEBACKUP_PHASE_ESTIMATE_BACKUP_SIZE. Oh, good point. v2 attached.

Re: tablespace_map code cleanup

2020-05-04 Thread Amit Kapila
On Wed, Apr 29, 2020 at 9:57 PM Robert Haas wrote: > > Hi, > > I think it's not good that do_pg_start_backup() takes a flag which > tells it to call back into basebackup.c's sendTablespace(). This means > that details which ought to be private to basebackup.c leak out and > become visible to other