Re: Calling pgstat_report_wait_end() before ereport(ERROR)

From: Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>
To: Michael Paquier <michael(at)paquier(dot)xyz>
Cc: PostgreSQL-development <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Calling pgstat_report_wait_end() before ereport(ERROR)
Date: 2019-04-12 13:06:41
Message-ID: CAD21AoD0S5HvpMbPfqNOn2W3AU2vxzYHJcDPHrU3krm63JhJTQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Apr 12, 2019 at 9:07 PM Michael Paquier <michael(at)paquier(dot)xyz> wrote:
>
> On Fri, Apr 12, 2019 at 07:27:44PM +0900, Masahiko Sawada wrote:
> > As far as I know there are three places where call
> > pgstat_report_wait_end before ereport(ERROR): two in twophase.c
> > andanother in copydir.c(at L199). 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?
>
> RecreateTwoPhaseFile() gets called in the checkpointer or the startup
> process which do not have a transaction context

Yes.

> so the wait event would not get cleaned up

But I think that's not right, I've checked the code. If the startup
process failed in that function it raises a FATAL and recovery fails,
and if checkpointer process does then it calls
pgstat_report_wait_end() in CheckpointerMain().

> It looks that 249cf070 has been rather
> inconsistent in its way of handling things.

Yeah, I think that at least handling of pgstat_report_wait_end() in
RecreateTwoPhseFile() is inconsistent in any case.

Regards,

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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2019-04-12 13:12:11 Re: improve PQexec documentation
Previous Message Fred .Flintstone 2019-04-12 13:04:34 Re: PostgreSQL pollutes the file system