On Fri, Mar 17, 2017 at 3:38 AM, Michael Banck
<michael.banck@credativ.de> wrote:
>> Your patch would work with the stream mode though.
>
> Or, if not requesting the "WAL" option of the replication protocol's
> BASE_BACKUP command.
>
> I agree it doesn't make sense to start messing with fetch mode, but I
> don't think we guarantee any ordering of tablespaces (to wit, Bernd was
> pretty sure it was the other way around all the time), neither do I
> think having the main tablespace be the first for non-WAL/stream and the
> last for WAL/fetch would be confusing personally, though I understand
> this is debatable.
From the docs:
https://www.postgresql.org/docs/9.6/static/protocol-replication.html
"After the second regular result set, one or more CopyResponse results
will be sent, one for the main data directory and one for each
additional tablespace other than pg_default and pg_global."
So yes there is no guarantee about the order tablespaces are sent.
> So I've updated the patch to only switch the main tablespace to be first
> in case WAL isn't included, please find it attached.
- /* Add a node for the base directory at the end */
+ /* Add a node for the base directory, either at the end or (if
+ * WAL is not included) at the beginning (if WAL is not
+ * included). This means the backup_label file is the first
+ * file to be sent in the latter case. */ ti = palloc0(sizeof(tablespaceinfo)); ti->size =
opt->progress? sendDir(".", 1, true, tablespaces,
true) : -1;
- tablespaces = lappend(tablespaces, ti);
+ if (opt->includewal)
+ tablespaces = lappend(tablespaces, ti);
+ else
+ tablespaces = lcons(ti, tablespaces);
The comment block format is incorrect. I would think as well that this
comment should say it is important to have the main tablespace listed
last it includes the WAL segments, and those need to contain all the
latest WAL segments for a consistent backup.
FWIW, I have no issue with changing the ordering of backups the way
you are proposing here as long as the comment of this code path is
clear.
--
Michael
