Date: Mon, 27 May 2019 06:56:02 -0700
From: Andres Freund <andres@anarazel.de>
To: Amit Khandekar <amitdkhan.pg@gmail.com>
Cc: tushar <tushar.ahuja@enterprisedb.com>,
	Petr Jelinek <petr.jelinek@2ndquadrant.com>,
	Robert Haas <robertmhaas@gmail.com>,
	Craig Ringer <craig@2ndquadrant.com>,
	Petr Jelinek <petr@2ndquadrant.com>,
	pgsql-hackers <pgsql-hackers@postgresql.org>
Subject: Re: Minimal logical decoding on standbys
Message-ID: <20190527135602.dw6cp5vttbdb76i7@alap3.anarazel.de>
References: <20190412192747.m73h2qnddrpzefij@alap3.anarazel.de>
 <CAJ3gD9fNuB9hTBMmQ=k9Be0LmPVEPwb8nv-5jc=fbgxUn=82Vw@mail.gmail.com>
 <20190521161937.tpepqkiatw6kv35v@alap3.anarazel.de>
 <CAJ3gD9c9EP6CGB+hGoDJKeVYQG67mtWcgVRhOAhVhh44hOBkmw@mail.gmail.com>
 <20190523155901.acm4umrl6adrcwt4@alap3.anarazel.de>
 <CAJ3gD9dKw8OEDjwbTmQ6e3-VGY64keNRxycHF4roJpsqhtssuw@mail.gmail.com>
 <20190523174821.s3kgumgmo35opavg@alap3.anarazel.de>
 <CAJ3gD9dv60f-QE21OQkZh3Wc1MCXrkVg=Yz5oPLzAmj-DQAeBw@mail.gmail.com>
 <CAJ3gD9fJdB6yZ5SL_h7GvVZxCLA7kXVpNzwjvggPge7kHZ1Bsw@mail.gmail.com>
 <CAJ3gD9d3uyK2iaO+PD=64HvOBerZPz3JyaPc6qF=kVZ_jGvgeQ@mail.gmail.com>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: 
 <CAJ3gD9d3uyK2iaO+PD=64HvOBerZPz3JyaPc6qF=kVZ_jGvgeQ@mail.gmail.com>
Precedence: bulk

On 2019-05-27 17:04:44 +0530, Amit Khandekar wrote:
> On Fri, 24 May 2019 at 21:00, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> >
> > On Fri, 24 May 2019 at 19:26, Amit Khandekar <amitdkhan.pg@gmail.com> wrote:
> > > Working on the patch now ....
> >
> > Attached is an incremental WIP patch
> > handle_wal_level_changes_WIP.patch to be applied over the earlier main
> > patch logical-decoding-on-standby_v4_rebased.patch.
> 
> I found an issue with these changes : When we change master wal_level
> from logical to hot_standby, and again back to logical, and then
> create a logical replication slot on slave, it gets created; but when
> I do pg_logical_slot_get_changes() with that slot, it seems to read
> records *before* I created the logical slot, so it encounters
> parameter-change(logical=>hot_standby) record, so returns an error as
> per the patch, because now in DecodeXLogOp() I error out when
> XLOG_PARAMETER_CHANGE is found :


> @@ -190,11 +190,23 @@ DecodeXLogOp(LogicalDecodingContext *ctx,
> XLogRecordBuffer *buf)
>              * can restart from there.
>              */
>             break;
> +         case XLOG_PARAMETER_CHANGE:
> +         {
> +           xl_parameter_change *xlrec =
> +             (xl_parameter_change *) XLogRecGetData(buf->record);
> +
> +           /* Cannot proceed if master itself does not have logical data */
> +           if (xlrec->wal_level < WAL_LEVEL_LOGICAL)
> +             ereport(ERROR,
> +                 (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> +                  errmsg("logical decoding on standby requires "
> +                     "wal_level >= logical on master")));
> +           break;
> +         }
> 
> I thought it won't read records *before* the slot was created. Am I
> missing something ?

That's why I had mentioned that you'd need to adapt
ReplicationSlotReserveWal(), to use the replay LSN or such.

Greetings,

Andres Freund