Re: Calling pgstat_report_wait_end() before ereport(ERROR)

On Fri, Apr 12, 2019 at 11:05 PM Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
>
> Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com> writes:
> > There are something like the following code in many places in PostgreSQL code.
> > ...
> > Since we eventually call
> > pgstat_report_wait_end() in AbortTransaction(). I think that we don't
> > need to call pgstat_report_wait_end() if we're going to raise an error
> > just after that. Is that right?
>
> Yes ... and those CloseTransientFile calls are unnecessary as well.
>
> To a first approximation, *any* cleanup-type call occurring just before
> an ereport(ERROR) is probably unnecessary, or if it is necessary then
> the code is broken in other ways. One should not assume that there is
> no other way for an error to be thrown while the resource is held, and
> therefore it's generally better design to have enough infrastructure
> so that the error cleanup mechanisms can handle whatever cleanup is
> needed. We certainly have such infrastructure for OpenTransientFile/
> CloseTransientFile, and according to what you say above (I didn't
> check it) pgstat wait reporting is handled similarly. So these
> call sites could all be simplified substantially.
>
> There are exceptions to this rule of thumb. In some places, for
> instance, it's worth releasing a lock before ereport simply to shorten
> the length of time that the lock might stay held. And there are places
> where a very low-level resource (such as a spinlock) is only held in
> straight-line code so there's not really need for error cleanup
> infrastructure for it. Perhaps there's an argument to be made that
> pgstat wait reporting could be put in this second category, but
> I doubt it.
>

Thank you for explanation! That's really helpful for me.

Regards,

--
Masahiko Sawada
NIPPON TELEGRAPH AND TELEPHONE CORPORATION
NTT Open Source Software Center