On 11 Feb 2021, at 10:42, Amit Kapila <amit.kapil...@gmail.com> wrote:
> 
> On Thu, Feb 11, 2021 at 1:51 PM Petr Jelinek
> <petr.jeli...@enterprisedb.com> wrote:
>> 
>> On 10 Feb 2021, at 06:32, Amit Kapila <amit.kapil...@gmail.com> wrote:
>>> 
>>> On Wed, Feb 10, 2021 at 7:41 AM Peter Smith <smithpb2...@gmail.com> wrote:
>>>> 
>>>> On Tue, Feb 9, 2021 at 10:38 AM Peter Smith <smithpb2...@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 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.

—
Petr



Reply via email to