From: | Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> |
---|---|
To: | Petr Jelinek <petr(dot)jelinek(at)enterprisedb(dot)com> |
Cc: | Peter Smith <smithpb2250(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | Re: Single transaction in the tablesync worker? |
Date: | 2021-02-11 09:56:20 |
Message-ID: | CAA4eK1K3YahYLH=r=K=cZ54hh7M88whm9E0+Wb0sKXjM5xE8PQ@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
On Thu, Feb 11, 2021 at 3:20 PM Petr Jelinek
<petr(dot)jelinek(at)enterprisedb(dot)com> wrote:
>
> On 11 Feb 2021, at 10:42, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >
> > On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
> > <petr(dot)jelinek(at)enterprisedb(dot)com> wrote:
> >>
> >> On 10 Feb 2021, at 06:32, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> >>>
> >>> On Wed, Feb 10, 2021 at 7:41 AM Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> >>>>
> >>>> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith <smithpb2250(at)gmail(dot)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 does not 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?
>
> Well, I was thinking it could be NOTICE or LOG to be honest, WARNING seems unnecessarily scary for those usecases to me.
>
I am fine with LOG and will make that change. Do you have any more
comments or want to spend more time on this patch before we call it
good?
--
With Regards,
Amit Kapila.
From | Date | Subject | |
---|---|---|---|
Next Message | Petr Jelinek | 2021-02-11 10:02:31 | Re: Single transaction in the tablesync worker? |
Previous Message | Petr Jelinek | 2021-02-11 09:50:56 | Re: Single transaction in the tablesync worker? |