Date: Fri, 4 Nov 2016 19:47:24 +0530
From: Abhijit Menon-Sen <ams@2ndQuadrant.com>
To: Simon Riggs <simon@2ndquadrant.com>
Cc: Michael Paquier <michael.paquier@gmail.com>,
	Robert Haas <robertmhaas@gmail.com>, pgsql-hackers@postgresql.org
Subject: Re: Proposal for changes to recovery.conf API
Message-ID: <20161104141724.GA20702@toroid.org>
References: 
 <CANP8+jLO5fmfudbB1b1iw3pTdOK1HBM=xMTaRfOa5zpDVcqzew@mail.gmail.com>
 <CA+TgmoZ0XewcJAOO5a9gCKoAdpt=wLxEY0i3vWA=ZBLY-v345w@mail.gmail.com>
 <77122541-8ef8-89a5-b8f7-361af007cd02@pgmasters.net>
 <CA+Tgmobn=iOFfbEoZNE4=TYDFB1-Ka489PistSHYaT1oChsMGg@mail.gmail.com>
 <20161031234458.GA17299@toroid.org>
 <CA+TgmoZeStE9QUgtZgXvD_no1p3EU7iU0YxDHQJaAt0TuJzk0g@mail.gmail.com>
 <CAB7nPqQmqKQhpFkfYSRM7+AiJoECKehpmYTip02zdNn16Dvo_w@mail.gmail.com>
 <CANP8+j+1p7sUKwTchpkQmpnddYutcqq9LYjEVd6_e9_9mgPjEw@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=utf-8
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: 
 <CANP8+j+1p7sUKwTchpkQmpnddYutcqq9LYjEVd6_e9_9mgPjEw@mail.gmail.com>
Precedence: bulk
Sender: pgsql-hackers-owner@postgresql.org

At 2016-11-04 10:35:05 +0000, simon@2ndquadrant.com wrote:
>
> Since the "lots of parameters" approach is almost exactly what we have
> now, I think we should do that first, get this patch committed and
> then sit back and discuss an improved API and see what downsides it
> introduces.

Thanks, I agree strongly with this.

Someone (Michael?) earlier mentioned the potential for introducing bugs
with this patch (the idea of merging recovery.conf into postgresql.conf
at all, not this particular incarnation).

I think the current proposed approach with

    recovery_target=xid
    recovery_target_xid=xxx

is preferable because (a) it doesn't introduce much new code, e.g., new
parsing functions, nor (b) need many changes in the documentation—all we
need is to say that of the various recovery_target_xxx parameters, the
one that has priority is the one named by recovery_target.

If I were to introduce recovery_target='xid xxx', I would at a minimum
need to factor out (or duplicate) parsing and error handling code, make
a type/value union-in-struct to store in the GUC *extra, then make sure
that we handle the older values in a backwards-compatible way, and move
a bunch of documentation around.

Can it be done? Yes, of course; and I'll do it if that's the consensus.
But it would be a backwards-compatible change to the current approach
anyway, and I think it would be better to put in the simple way now
before discussing the new API further.

Let's get the basic new *functionality* out so that people can play with
it, I'm sure we'll find a few non-API things that need tweaking as a
result of the change.

-- Abhijit


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers