Re: WIP/PoC for parallel backup

On Wed, Oct 30, 2019 at 10:16 AM Asif Rehman <asifr(dot)rehman(at)gmail(dot)com> wrote:
> 'startptr' is used by sendFile() during checksum verification. Since
> SendBackupFiles() is using sendFIle we have to set a valid WAL location.

Ugh, global variables.

Why are START_BACKUP, SEND_BACKUP_FILELIST, SEND_BACKUP_FILES, and
STOP_BACKUP all using the same base_backup_opt_list production as
BASE_BACKUP? Presumably most of those options are not applicable to
most of those commands, and the productions should therefore be
separated.

You should add docs, too. I wouldn't have to guess what some of this
stuff was for if you wrote documentation explaining what this stuff
was for. :-)

>> The tablespace_path option appears entirely unused, and I don't know
>> why that should be necessary here, either.
>
> This is to calculate the basepathlen. We need to exclude the tablespace location (or
> base path) from the filename before it is sent to the client with sendFile call. I added
> this option primarily to avoid performing string manipulation on filename to extract the
> tablespace location and then calculate the basepathlen.
>
> Alternatively we can do it by extracting the base path from the received filename. What
> do you suggest?

I don't think the server needs any information from the client in
order to be able to exclude the tablespace location from the pathname.
Whatever it needs to know, it should be able to figure out, just as it
would in a non-parallel backup.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company