On Thursday, December 2, 2021 1:49 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> On Thu, Dec 2, 2021 at 6:35 AM osumi.takamichi@fujitsu.com
> <osumi.takamichi@fujitsu.com> wrote:
> >
> > On Wednesday, December 1, 2021 10:16 PM Amit Kapila
> <amit.kapila16@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