On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
<petr.jelinek@enterprisedb.com> wrote:
>
> On 10 Feb 2021, at 06:32, Amit Kapila <amit.kapila16@gmail.com> wrote:
> >
> > On Wed, Feb 10, 2021 at 7:41 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >>
> >> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >>>
> >>
> >> PSA v2 of this WalRcvExceResult patch (it is same as v1 but includes
> >> some PG doc updates).
> >> This applies OK on top of v30 of the main patch.
> >>
> >
> > Thanks, I have integrated these changes into the main patch and
> > additionally made some changes to comments and docs. I have also fixed
> > the function name inconsistency issue you reported and ran pgindent.
>
> One thing:
>
> > + else if (res->status == WALRCV_ERROR &&
> > + missing_ok &&
> > + res->sqlstate == ERRCODE_UNDEFINED_OBJECT)
> > + {
> > + /* WARNING. Error, but missing_ok = true. */
> > + ereport(WARNING,
> > (errmsg("could not drop the replication slot \"%s\" on publisher",
> > slotname),
> > errdetail("The error was: %s", res->err)));
>
> Hmm, why is this WARNING, we mostly call it with missing_ok = true when the slot is not expected to be there, so it
doesnot seem correct to report it as warning?
>
WARNING is for the cases where we don't always expect slots to exist
and we don't want to stop the operation due to it. For example, in
DropSubscription, for some of the rel states like (SUBREL_STATE_INIT
and SUBREL_STATE_DATASYNC), the slot won't exist. Similarly, say if we
fail (due to network error) after removing some of the slots, next
time, it will again try to drop already dropped slots and fail. For
these reasons, we need to use WARNING. Similarly for tablesync workers
when we are trying to initially drop the slot there is no certainty
that it exists, so we can't throw ERROR and stop the operation there.
There are other cases like when the table sync worker has finished
syncing the table, there we will raise an ERROR if the slot doesn't
exist. Does this make sense?
--
With Regards,
Amit Kapila.