Обсуждение: [HACKERS] Reporting query on crash even if completed

Hi,

When a backend dies, in a manner triggering a crash restart, we
currently log something like:

LOG: %s (PID %d) was terminated by signal %d
LOG: %s (PID %d) exited with exit code %d
DETAIL: Failed process was running: %s

Notably we log the query from the backend whenever
pgstat_get_crashed_backend_activity() can find a query string.

That used to be only when there's an active query, but since
commit 4f42b546fd87a80be30c53a0f2c897acb826ad52
Author: Magnus Hagander <magnus@hagander.net>
Date:   2012-01-19 14:19:20 +0100
   Separate state from query string in pg_stat_activity
   This separates the state (running/idle/idleintransaction etc) into   it's own field ("state"), and leaves the query
fieldcontaining just   query text.
 
   The query text will now mean "current query" when a query is running   and "last query" in other states.
Accordingly,thefield has been   renamed from current_query to query.
 
   Since backwards compatibility was broken anyway to make that, the procpid   field has also been renamed to pid -
alongwith the same field in   pg_stat_replication for consistency.
 
   Scott Mead and Magnus Hagander, review work from Greg Smith

that's not the case anymore. I can't recall anybody complaining, but to
me it seems fairly confusing to report that some query was running when
it's actually not.

Magnus, was that intentional?  Others, does anybody think we want it
this way?


Both manually signalling and the OOM killer probably are the biggest
source for backends being killed even when there's no queries currently
running.

Greetings,

Andres Freund


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Andres Freund <andres@anarazel.de> writes:
> When a backend dies, in a manner triggering a crash restart, we
> currently log something like:
> DETAIL: Failed process was running: %s
> 
> That used to be only when there's an active query, but since
> commit 4f42b546fd87a80be30c53a0f2c897acb826ad52
> that's not the case anymore. I can't recall anybody complaining, but to
> me it seems fairly confusing to report that some query was running when
> it's actually not.

I think this is fine.  If the backend crashes in nominally-post-query
mopup, it might be useful to know what it had been running.  If it
really was idle, it seems no more confusing than in pg_stat_activity.

Now, for pg_stat_activity part of the argument why this wouldn't be
confusing was that you could also see the "state" field.  Maybe we
should try to shoehorn equivalent info into the crash log entry?
        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

On Mon, Sep 18, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Now, for pg_stat_activity part of the argument why this wouldn't be
> confusing was that you could also see the "state" field.  Maybe we
> should try to shoehorn equivalent info into the crash log entry?

Yeah, I think so.  Really, I think this is an inadvertency, and thus a
bug.  But instead of just not showing the query when the backend is
idle, I'd change the display for that case to:

DETAIL: Failed process was idle; last query was: %s

Or something like that.  I guess we'd need another case for a backend
that crashed without ever running a query.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Robert Haas <robertmhaas@gmail.com> writes:
> On Mon, Sep 18, 2017 at 9:47 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Now, for pg_stat_activity part of the argument why this wouldn't be
>> confusing was that you could also see the "state" field.  Maybe we
>> should try to shoehorn equivalent info into the crash log entry?

> Yeah, I think so.  Really, I think this is an inadvertency, and thus a
> bug.  But instead of just not showing the query when the backend is
> idle, I'd change the display for that case to:

> DETAIL: Failed process was idle; last query was: %s

WFM.

> Or something like that.  I guess we'd need another case for a backend
> that crashed without ever running a query.

We already print nothing in that case, which seems fine.
        regards, tom lane


-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers