Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Alternative to backup-config files #81

Closed
jberkus opened this issue Oct 26, 2015 · 8 comments
Closed

Alternative to backup-config files #81

jberkus opened this issue Oct 26, 2015 · 8 comments

Comments

@jberkus
Copy link
Contributor

jberkus commented Oct 26, 2015

I wanted to suggest an atlernative to the current backup config files functionality on that branch.

We could support a conf.d directory, which would be outside PGDATA, and thus not wiped out by restores.

I believe this is a much better route than backing up and restoring conf files:

  1. we don't have to touch those files, so they could be maintained by a CMF
  2. doesn't involve a bunch of copying files
  3. makes it easier to extend functionality by simply dropping files in the conf.d directory

... in fact, I plan to implement conf.d support anyway, so why not just go that way?

If there's some reason why you also need pg.conf backup, then can we please make it an optional feature? For container-based deployments, it makes no sense to be "backing up" pg.conf etc.

@alexeyklyukin
Copy link
Contributor

We don't backup postgresql.conf in order preserve it for the future: there are only 2 reasons why are we doing this:

  1. wal-e removes all .conf files when it restores the backup.
  2. pg_basebackup does not copy symlinks, in order to create the replica out of the master with symlinked .conf files we have to copy the symlinks into normal files inside PGDATA, then produce the base backup and then copy them back.

The first reason is the primary reason Patroni produces the .backups, the second is just a side effect that solves the issue.

I don't have any issues with supporting conf.d configuration (you didn't specify any details, so I guess it would be implemented by supplying command-line options to pg_ctl in order to specify the location of the configuration files), as long as it is not a default option and Patroni can still locate the recovery.conf in order to rewrite it during promition/demotion.

@jberkus
Copy link
Contributor Author

jberkus commented Oct 27, 2015

Ah, OK, I didn't understand this!

  1. should then be part of the WAL-E backup module, not part of main Patroni, no?
  2. Do people really use symlinked .conf files?

My main point is that backing up the .conf files is currently not optional in the code; it happens whether you actually need it or not. Since most installations don't need it, it should be optional, and off by default.

For conf.d, we need to support that by appending to the pg.conf file. You cannot supply a pg_ctl option to add a conf directory; that can only be done in the .conf file. That's why I haven't implemented it yet; writing to pg.conf is a new thing for us.

@alexeyklyukin
Copy link
Contributor

I don't understand what is the problem with conf.backup files: we don't overwrite existing configuration files in PGDATA, so the only case Patroni decides to copy over the conf.backup files is when the .conf files are actually missing from the PGDATA, but present in the backup: the latter means they were in the original PGDATA that were copied into the backup, therefore that PostgreSQL cluster likely used configuration files in PGDATA. If configuration files are stored outside of PGDATA - no con.backup files will be created.

WAL-E restore will be broken without copying/restoring the configuration files (as the replica created from WAL-E won't be able to start), so it should be turned on at least when wal-e is chosen as a replica method. We also symlink configuration files (we keep configuration files in git for all PostgreSQL clusters in our data centre and link them to actual PGDATA) internally, so removal of this code will break Patroni for us.

Nevertheless, we can live with this functionality being optional, but I want to learn if there is an actual use case where it breaks the production system? Perhaps it would make sense to enable it automatically when WAL-E is set as a replication method, or when configuration files are symlinked?

For the conf.d, one can already point patroni to the existing postgresql.conf with the options described at #53. The only step is to create postgresql.conf, with include_dir pointing somewhere, but why should it be the task of Patroni? A docker container containing the postgresql.conf with include_dir should be good enough to make sure the same configuration is propagated to the replica.

@jberkus
Copy link
Contributor Author

jberkus commented Oct 28, 2015

I've had failures because of permissions issues with the backup file location. When the user doesn't need conf file backups to begin with, they shouldn't be asked to troubleshoot those failures.

The reason why patroni needs to add the conf.d line to pg.conf is that on clean deploys, patroni is running initdb to begin with, which means that patroni is generating the pg.conf. There is no opportunity for any other utility to modify pg.conf in any way.

@lasomethingsomething
Copy link
Contributor

@jberkus @feikesteenbergen @alexeyklyukin What would be the next step here?

@alexeyklyukin
Copy link
Contributor

@LappleApple @jberkus I think it's a valid request. Let's keep this ticket until someone of us has time to implement it.

@CyberDem0n
Copy link
Member

I believe this topic is pretty much covered by #466

@jberkus
Copy link
Contributor Author

jberkus commented Aug 9, 2017

yep, thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants