Greetings, * Peter Eisentraut (peter.eisentr...@2ndquadrant.com) wrote: > Committed with that addition.
I'm concerned that this hasn't been entirely thought through regarding pretty common use-cases, consider a pretty typical pg_basebackup -R usage case: - User performs a backup with pg_basebackup -R - Replica is then promoted to be a primary - User performs a backup with pg_basebackup -R on the new primary - Duplicate entries end up in postgresql.auto.conf. Ew. Now thinking about it from the perspective of... more complete backup solutions, which give the user the option to fill out more of what used to go into recovery.conf, it seems like there's going to have to be a lot more hacking around with postgresql.auto.conf, such as adding in recovery target time and other parameters, which I'm not terribly thrilled with. For example, when a backup is done, typically postgresql.auto.conf comes along with it and gets restored with it, but if that's done now without mucking with the contents, we'll run into the same issue that pg_basebackup has as outlined above- recovery target time and other parameters being included in the postgresql.auto.conf, possibly multiple times if care isn't taken to avoid that. While it used to be that you could end up with recovery.done files ending up in backups, they were just a nuisance and didn't impact anything. One possible approach for dealing with at least some of these concerns would be to remove the recovery settings from postgresql.auto.conf once recovery is done, which is similar to how we moved recovery.conf to recovery.done, at least. We could even create a recovery.done file if we wanted to. Unfortunately, this also means that users are going to have their own issues when just using pg_basebackup and trying to perform PITR since they'll be modifying postgresql.conf and will have to remember to remove those settings since we aren't going to modify that. Maybe we should have a warning or notice or something saying "hey, now that this has been promoted, maybe you should check your config file and clear out the recovery settings?". Thinking about how this would work with something like pgbackrest dropping settings into postgresql.auto.conf, users would need to be educated to use ALTER SYSTEM if they want to move the recovery target time to be later while doing PITR (to avoid having them hacking around in postgresql.auto.conf), which is perhaps not terrible. What would make that experience nicer would be a way to restart PG remotely after making that change (well, or just making it so that users could tell PG to continue to play forward, but as I understand it this patch doesn't give us that). These are my thoughts on this, at least at the moment. I've also asked David Steele to comment, of course, since this will impact pgbackrest. In the end, I'm not entirely convinced that eliminating recovery.conf is actually an improvement; I think I would have rather seen the original approach of having an 'auto' recovery.conf, but perhaps we can improve on this since others seemed anxious to get rid of recovery.conf (though I'm not sure why- seems like we'll likely end up with more code to handle things cleanly with a merged recovery.auto/postgresql.auto than if we had kept them independent). Thanks! Stephen
signature.asc
Description: PGP signature
