Re: Minimal logical decoding on standbys

On Mon, Apr 3, 2023 at 12:05 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Mon, Apr 3, 2023 at 4:39 AM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >
> > > From 56a9559555918a99c202a0924f7b2ede9de4e75d Mon Sep 17 00:00:00 2001
> > > From: bdrouvotAWS <bdrouvot@amazon.com>
> > > Date: Tue, 7 Feb 2023 08:59:47 +0000
> > > Subject: [PATCH v52 3/6] Allow logical decoding on standby.
> > >
> > > Allow a logical slot to be created on standby. Restrict its usage
> > > or its creation if wal_level on primary is less than logical.
> > > During slot creation, it's restart_lsn is set to the last replayed
> > > LSN. Effectively, a logical slot creation on standby waits for an
> > > xl_running_xact record to arrive from primary.
> >
> > Hmm, not sure if it really applies here, but this sounds similar to
> > issues with track_commit_timestamps: namely, if the primary has it
> > enabled and you start a standby with it enabled, that's fine; but if the
> > primary is later shut down (but the standby isn't) and then the primary
> > restarted with a lesser value, then the standby would misbehave without
> > any obvious errors.
> >
>
> IIUC, the patch deals it by invalidating logical slots while replaying
> the XLOG_PARAMETER_CHANGE record on standby. Then later during
> decoding, if it encounters XLOG_PARAMETER_CHANGE, and wal_level from
> primary has been reduced, it will return an error. There is a race
> condition here as explained in the patch as follows:
>
> + /*
> + * If wal_level on primary is reduced to less than logical, then we
> + * want to prevent existing logical slots from being used.
> + * Existing logical slots on standby get invalidated when this WAL
> + * record is replayed; and further, slot creation fails when the
> + * wal level is not sufficient; but all these operations are not
> + * synchronized, so a logical slot may creep in while the wal_level
> + * is being reduced. Hence this extra check.
> + */
> + 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")));
>
> Now, during this race condition, say not only does a logical slot
> creep in but also one tries to decode WAL using the same then some
> misbehavior is expected. I have not tried this so not sure if this is
> really a problem but are you worried about something along those
> lines?
>

On further thinking, as such this shouldn't be a problem because all
the WAL records before PARAMETER_CHANGE record will have sufficient
information so that they can get decoded. However, with the current
approach, the subscriber may not even receive the valid records before
PARAMETER_CHANGE record. This is because startup process will
terminate the walsenders while invaliding the slots and after restart
the walsenders will exit because the corresponding slot will be an
invalid slot. So, it is quite possible that walsender was lagging and
wouldn't have sent records before the PARAMETER_CHANGE record making
subscriber never receive those records that it should have received. I
don't know whether this is what one would expect.

One other observation is that once this error has been raised both
standby and subscriber will keep on getting this error in the loop
unless the user manually disables the subscription on the subscriber.

-- 
With Regards,
Amit Kapila.