Re: WIP/PoC for parallel backup

From: Asif Rehman <asifr(dot)rehman(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: WIP/PoC for parallel backup
Date: 2019-11-04 13:08:55
Message-ID: CADM=JegGcwi=zr4wjkB+EnrxzQTmbeK6Zti6bYyNo22syF18uA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Nov 1, 2019 at 8:53 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> 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.
>

Are you expecting something like the attached patch? Basically I have
reorganised the grammar
rules so each command can have the options required by it.

I was feeling a bit reluctant for this change because it may add some
unwanted grammar rules in
the replication grammar. Since these commands are using the same options as
base backup, may
be we could throw error inside the relevant functions on unwanted options?

> 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. :-)
>

Yes I will add it in the next patch.

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

--
Asif Rehman
Highgo Software (Canada/China/Pakistan)
URL : www.highgo.ca

Attachment Content-Type Size
repl_grammar.patch application/octet-stream 6.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-11-04 13:25:59 Re: auxiliary processes in pg_stat_ssl
Previous Message Grigory Smolkin 2019-11-04 13:03:38 [proposal] recovery_target "latest"