On 2/20/20 12:55 AM, Michael Paquier wrote:
On Wed, Feb 19, 2020 at 12:37:00PM -0600, David Steele wrote:
As far as I can see, the pg_internal.init.XX will not be cleaned up by
PostgreSQL on startup. I've only tested this in 9.6 so far, but I don't see
any differences in the code since then that would lead to better behavior.
Perhaps that's also something we should fix?
Not sure that it is worth spending cycles on that at the beginning of
recovery as when a mapping file is written its temporary entry
truncates any existing one present matching its name.
But since the name includes the backend's pid you would need to get
lucky and have a new backend with the same pid create the file after a
restart. I tried it and the old temp file was left behind after restart
and first connection to the database.
I doubt this is a big issue in the field, but it seems like it would be
nice to do something about it.
I'm really not a fan of a blind prefix match. I think we should stick with
only excluding files that are created by Postgres.
Thinking more on that, excluding any backup_label with a custom suffix
worries me as it could cause a potential breakage for exiting backup
solutions. So attached is an updated patch making things in a
smarter way: I have added to the exclusion lists the possibility to
match an entry based on its prefix, or not, the choice being optional.
This solves the problem with pg_internal.PID and is careful to not
exclude unnecessary entries like suffixed backup labels or such. This
leads to some extra duplication within pg_rewind, basebackup.c and
pg_checksums but I think we can live with that, and that makes
back-patching simpler. Refactoring is still tricky though as it
relates to the use of paths across the backend and the frontend..
I'm not excited about the amount of code duplication between these three
tools. I know this was because of back-patching various issues in the
past, but I really think we need to unify these data
structures/functions in HEAD.
So backup_label.old and
tablespace_map.old should just be added to the exclude list. That's how we
have it in pgBackRest.
That would be a behavior change. We could change that on HEAD, but I
don't think that this can be back-patched as this does not cause an
actual problem.
Right, that should be in HEAD.
For now, my proposal is to fix the prefix first, and then let's look
at the business with tablespaces where needed.
OK.
Regards,
--
-David
da...@pgmasters.net
