Re: Minimal logical decoding on standbys

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 ?

>
> > >
> > > On 2019-05-21 09:19:37 -0700, Andres Freund wrote:
> > > > So I suspect we need conflict handling in xlog_redo's
> > > > XLOG_PARAMETER_CHANGE case. If we there check against existing logical
> > > > slots, we ought to be safe.
>
> Yet to do this. Andres, how do you want to handle this scenario ? Just
> drop all the existing logical slots like what we decided for conflict
> recovery for conflicting xids ?

I went ahead and added handling that drops existing slots when we
encounter XLOG_PARAMETER_CHANGE in xlog_redo().

Attached is logical-decoding-on-standby_v5.patch, that contains all
the changes so far.

-- 
Thanks,
-Amit Khandekar
EnterpriseDB Corporation
The Postgres Database Company