On Thursday, December 2, 2021 1:49 PM Amit Kapila <amit.kapil...@gmail.com>
wrote:
> On Thu, Dec 2, 2021 at 6:35 AM osumi.takami...@fujitsu.com
> <osumi.takami...@fujitsu.com> wrote:
> >
> > On Wednesday, December 1, 2021 10:16 PM Amit Kapila
> <amit.kapil...@gmail.com> wrote:
> > Updated the patch to include the notification.
> >
> The patch disables the subscription for non-transient errors. I am not sure
> if we
> can easily make the call to decide whether any particular error is transient
> or
> not. For example, DISK_FULL or OUT_OF_MEMORY might not rectify itself.
> Why not just allow to disable the subscription on any error? And then let the
> user check the error either in view or logs and decide whether it would like
> to
> enable the subscription or do something before it (like making space in disk,
> or
> fixing the network).
Agreed. I'll treat any errors as the trigger of the feature
in the next version.
> The other problem I see with this transient error stuff is maintaining the
> list of
> error codes that we think are transient. I think we need a discussion for
> each of
> the error_codes we are listing now and whatever new error_code we add in the
> future which doesn't seem like a good idea.
This is also true. The maintenance cost of my current implementation
didn't sound cheap.
> I think the code to deal with apply worker errors and then disable the
> subscription has some flaws. Say, while disabling the subscription if it
> leads to
> another error then I think the original error won't be reported. Can't we
> simply
> emit the error via EmitErrorReport and then do AbortOutOfAnyTransaction,
> FlushErrorState, and any other memory context clean up if required and then
> disable the subscription after coming out of catch?
You are right. I'll fix related parts accordingly.
Best Regards,
Takamichi Osumi
