Re: [HACKERS] WIP: Restricting pg_rewind to data/wal dirs

On Wed, Nov 1, 2017 at 5:58 PM, Chris Travers <chris(dot)travers(at)adjust(dot)com> wrote:
> I would also like to address a couple of important points here:
>
> 1. I think default restrictions plus additional paths is the best, safest
> way forward. Excluding shell-globs doesn't solve the "I need this
> particular config file" very well particularly if we want to support this
> outside of an internal environment. Shell globs also tend to be overly
> inclusive and so if you exclude based on them, you run into a chance that
> your rewind is corrupt for being overly exclusive.
>
> 2. I would propose any need for an additional paths be specified using an
> --Include-path directive. This could be specified multiple times and could
> point to a file or directory which would be added to the paths rewound. I
> have a patch for this mostly done but I am concerned that these sorts of
> changes result in a combination of changes that are easier to review
> separately than together. So if needed, I can add it or in a separate patch
> following.
>
> 3. I think it would be a mistake to tie backup solutions in non-replicated
> environments to replication use cases, and vice versa. Things like
> replication slots (used for streaming backups) have different considerations
> in different environments. Rather than build the same infrastructure first,
> I think it is better to support different use cases well and then build
> common infrastructure to support the different cases. I am not against
> building common infrastructure for pg_rewind and pg_basebackup. I am very
> much against having the core guarantees being the exact same.

+const char *rewind_dirs[] = {
+ "base", // Default tablespace
+ "global", // global tablespace
+ "pg_commit_ts", // In case we need to do PITR before up to sync
+ "pg_logical", // WAL related and no good reason to exclude
+ "pg_multixact", // WAL related and may need for vacuum-related reasons
+ "pg_tblspc", // Pther tablespaces
+ "pg_twophase", // mostly to *clear*
+ "pg_wal", // WAL
+ "pg_xact", // Commits of transactions
+ NULL
+};
Incorrect comment format here.

+ in full. The advantage of <application>pg_rewind</> over taking a new base
+ backup, or tools like <application>rsync</>, is that
<application>pg_rewind</>
This generates warnings on HEAD when compiling the documentation.

Please note that I am still -1 for using a methodology different than
what is used for base backups with an inclusive method, and would much
prefer an exclusive method by reusing the existing entries in
basebackup.c. Still, I am the only one who expressed an opinion about
this patch, so moved to next CF with waiting on author as status.
--
Michael