Re: Add an option to skip loading missing publication to avoid logical replication failure

On Thu, Mar 13, 2025 at 10:49 AM vignesh C <vignesh21(at)gmail(dot)com> wrote:
>
> On Thu, 13 Mar 2025 at 09:18, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
> >
> > Thanks looks good to me.
> >
> > While looking at the patch, I have a few comments/questions
> >
> > + if (pub)
> > + result = lappend(result, pub);
> > + else
> > + {
> > + /*
> > + * When executing 'ALTER SUBSCRIPTION ... SET PUBLICATION', the
> > + * apply worker continues using the existing replication slot and
> > + * origin after restarting. If the replication origin is not
> > + * updated before the restart, the WAL start location may point to
> > + * a position before the specified publication exists, causing
> > + * persistent apply worker restarts and errors.
> > + *
> > + * This ensures that the publication is skipped if it does not
> > + * exist and is loaded when the corresponding WAL record is
> > + * encountered.
> > + */
> > + ereport(WARNING,
> > + errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> > + errmsg("skipped loading publication: %s", pubname),
> > + errhint("If the publication already exists, ignore it as it will be
> > loaded upon reaching the corresponding WAL record; otherwise, create
> > it."));
> > + }
> >
> > This comment focuses on a specific use case regarding the problem with
> > 'ALTER SUBSCRIPTION ... SET PUBLICATION,' but in reality, we are
> > addressing a more general case where the user is trying to SET
> > PUBLICATION or even CREATE SUBSCRIPTION, and some publications are
> > missing. Wouldn't it be better to rephrase the comment?
>
> How about a comment something like below:
> /*
> * In 'ALTER SUBSCRIPTION ... ADD/SET PUBLICATION' and
> * 'CREATE SUBSCRIPTION', if the replication origin is not updated
> * before the worker exits, the WAL start location might point to a
> * position before the publication's WAL record. This can lead to
> * persistent apply worker restarts and errors.
> *
> * Additionally, dropping a subscription's publication should not
> * disrupt logical replication.
> *
> * This ensures that a missing publication is skipped and loaded
> * when its corresponding WAL record is encountered.
> */

Looks fine, shall we add the missing publication point as well
something like below

/*
* In operations like 'ALTER SUBSCRIPTION ... ADD/SET PUBLICATION' and
* 'CREATE SUBSCRIPTION', if the specified publication does not exist or
* if the replication origin is not updated before the worker exits,
* the WAL start location may point to a position prior to the publication's
* WAL record. This can cause persistent restarts and errors
* in the apply worker.
*
> * Additionally, dropping a subscription's publication should not
> * disrupt logical replication.
*
> * This ensures that a missing publication is skipped and loaded
> * when its corresponding WAL record is encountered.
*/

> ereport(WARNING,
> errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
> errmsg("skipped loading publication: %s", pubname),
> errhint("If the publication is missing, create and refresh it.
> Otherwise, wait for the slot to reach the WAL record, then refresh"));
>
> > 2. + errhint("If the publication already exists, ignore it as it will
> > be loaded upon reaching the corresponding WAL record; otherwise,
> > create it."));
> >
> > Is this hint correct? This is a question rather than a comment: When
> > we reach a particular WAL where the publication was created, will the
> > publication automatically load, or does the user need to REFRESH the
> > publications?
>
> Users need to refresh the publication in case the relation is not
> already added to pg_subscription_rel and apply incremental changes.
> How about an error hint like:
> "If the publication is missing, create and refresh it. Otherwise, wait
> for the slot to reach the WAL record for the created publication, then
> refresh"

+1

--
Regards,
Dilip Kumar
EnterpriseDB: http://www.enterprisedb.com