On 2015-01-03 12:07:29 +0200, Heikki Linnakangas wrote:
> >@@ -941,6 +936,8 @@ StartLogicalReplication(StartReplicationCmd *cmd)
> >      * Force a disconnect, so that the decoding code doesn't need to care
> >      * about an eventual switch from running in recovery, to running in a
> >      * normal environment. Client code is expected to handle reconnects.
> >+     * This covers the race condition where we are promoted half way
> >+     * through starting up.
> >      */
> >     if (am_cascading_walsender && !RecoveryInProgress())
> >     {
> 
> We could exit recovery immediately after this check. Why is this check
> needed?

I probably wrote that ched and I don't think it really is needed. I
think that's a remnant of what the physical pendant used to do.

I think this needs slightly more abstraction because the infrastructure
is local to walsender.c - but logical decoding is also possible via
SQL. I'm not yet sure how that should look like. It'd be awesome if in
the course of that we could get rid of the nearly duplicated XLogRead()
:(

Simon, have you checked that this actually correctly follows timelines?
Afaics the patch as is won't allow to start logical decoding on a standby.

To allow logical decoding from clients I (apperently) wrote the the
following comment:
        /* ----
         * TODO: We got to change that someday soon...
         *
         * There's basically three things missing to allow this:
         * 1) We need to be able to correctly and quickly identify the timeline 
a
         *        LSN belongs to
         * 2) We need to force hot_standby_feedback to be enabled at all times 
so
         *        the primary cannot remove rows we need.
         * 3) support dropping replication slots referring to a database, in
         *        dbase_redo. There can't be any active ones due to HS recovery
         *        conflicts, so that should be relatively easy.
         * ----
         */
        if (RecoveryInProgress())
                ereport(ERROR,
                                (errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
                           errmsg("logical decoding cannot be used while in 
recovery")));

You're implementing 1) here. 3) doesn't look very challenging.

But 2) imo is rather more interesting/complex. I guess we'd have to
force that streaming replication is used, that a physical replication
slot is used and that hot_standby_feedback is enabled.

Greetings,

Andres Freund

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


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

Reply via email to