Re: unite recovery.conf and postgresql.conf

On Fri, Sep 23, 2011 at 6:17 PM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Sep 23, 2011 at 12:51 PM, Josh Berkus <josh(at)agliodbs(dot)com> wrote:
>> I'm happy to make upgrades easier, but I want a path which eventually
>> ends in recovery.conf going away.  It's a bad API, confuses our users,
>> and is difficult to support and maintain.
>
> I agree.
>
> GUC = Grand Unified Configuration, but recovery.conf is an example of
> where it's not so unified after all.  We've already done a non-trivial
> amount of work to allow recovery.conf values to be specified without
> quotes, a random incompatibility with GUCs that resulted from having
> different parsing code for each file.  If that were the last issue,
> then maybe it wouldn't be worth worrying about, but it's not.  For
> example, it would be nice to have reload behavior on SIGHUP.
...
> I don't want us
> to have to implement such things separately for postgresql.conf and
> recovery.conf.

It was always my plan to do exactly the above, and there are code
comments that say that from 2004. The time to replace it is now and I
welcome that day and have already agreed to it.

We all want every word quoted above and nothing there is under debate.

> And we
> keep talking about having an ALTER SYSTEM SET guc = value or SET
> PERMANENT guc = value command, and I think ALTER SYSTEM SET
> recovery_target_time = '...' would be pretty sweet.  I don't want us
> to have to implement such things separately for postgresql.conf and
> recovery.conf.

There is a reason why it doesn't work that way which you overlook.
Please start a separate thread if you wish to discuss that.

> Now, it's true that Simon's proposal (of having recovery.conf
> automatically included) if it exists doesn't necessarily preclude
> those things.  But it seems to me that it is adding a lot of
> complexity to core for a pretty minimal benefit to end-users, and that
> the semantics are not going to end up being very clean.  For example,
> now you potentially have the situation where recovery.conf has
> work_mem=128MB and postgresql.conf has work_mem=4MB, and now when you
> end recovery you've got to make sure that everyone picks up the new
> setting.  Now, in some sense you could say that's a feature addition,
> and I'm not going to deny that it might be useful to some people, but
> I think it's also going to require a fairly substantial convolution of
> the GUC machinery, and it's going to discourage people from moving
> away from recovery.conf.  And like Josh, I think that ought to be the
> long-term goal, for the reasons he states.

The semantics are clear: recovery.conf is read first, then
postgresql.conf. It's easy to implement (1 line of code) and easy to
understand.

So we can support the old and the new very, very easily and clearly.
"Complexity" - no definitely not. "Minimal benefit for end users" -
backwards compatibility isn't minimal benefit. It's a major issue.

If you put things in two places, yes that causes problems. You can
already add the same parameter twice and cause exactly the same
problems.

> I don't want to go willy-nilly breaking third-party tools that work
> with PostgreSQL, but in this case I think that the reason there are so
> many tools in the first place is because what we're providing in core
> is not very good.  If we are unwilling to improve it for fear of
> breaking compatibility with the tools, then we are stuck.

No, there are many tools because there are many requirements. A
simple, open API has allowed our technology to be widely used. That
was by design not by chance.

Nobody is unwilling to improve it. The debate is about people being
unwilling to provide a simple and easy to understand backwards
compatibility feature, which breaks things for no reason and does not
interfere with the proposed new features.

--
 Simon Riggs                   http://www.2ndQuadrant.com/
 PostgreSQL Development, 24x7 Support, Training & Services