Log the location field before any backtrace

Started by Peter Eisentrautover 5 years ago5 messages
#1Peter Eisentraut
peter.eisentraut@2ndquadrant.com
1 attachment(s)

In PG13, we added the ability to add backtraces to the log output.
After some practical experience with it, I think the order in which the
BACKTRACE and the LOCATION fields are printed is wrong. I propose we
put the LOCATION field before the BACKTRACE field, not after. This
makes more sense because the location is effectively at the lowest level
of the backtrace.

Patch attached.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

Attachments:

0001-Log-the-location-field-before-any-backtrace.patchtext/plain; charset=UTF-8; name=0001-Log-the-location-field-before-any-backtrace.patch; x-mac-creator=0; x-mac-type=0Download
From aaf4a8146cf60a5959e50432359fd9972f410e1b Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <peter@eisentraut.org>
Date: Thu, 9 Jul 2020 11:00:34 +0200
Subject: [PATCH] Log the location field before any backtrace

This order makes more sense because the location is effectively at the
lowest level of the backtrace.
---
 src/backend/utils/error/elog.c | 14 +++++++-------
 1 file changed, 7 insertions(+), 7 deletions(-)

diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index e976201030..e4b717c79a 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -2938,13 +2938,6 @@ send_message_to_server_log(ErrorData *edata)
 			append_with_tabs(&buf, edata->context);
 			appendStringInfoChar(&buf, '\n');
 		}
-		if (edata->backtrace)
-		{
-			log_line_prefix(&buf, edata);
-			appendStringInfoString(&buf, _("BACKTRACE:  "));
-			append_with_tabs(&buf, edata->backtrace);
-			appendStringInfoChar(&buf, '\n');
-		}
 		if (Log_error_verbosity >= PGERROR_VERBOSE)
 		{
 			/* assume no newlines in funcname or filename... */
@@ -2962,6 +2955,13 @@ send_message_to_server_log(ErrorData *edata)
 								 edata->filename, edata->lineno);
 			}
 		}
+		if (edata->backtrace)
+		{
+			log_line_prefix(&buf, edata);
+			appendStringInfoString(&buf, _("BACKTRACE:  "));
+			append_with_tabs(&buf, edata->backtrace);
+			appendStringInfoChar(&buf, '\n');
+		}
 	}
 
 	/*
-- 
2.27.0

#2Daniel Gustafsson
daniel@yesql.se
In reply to: Peter Eisentraut (#1)
Re: Log the location field before any backtrace

On 9 Jul 2020, at 11:17, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

In PG13, we added the ability to add backtraces to the log output. After some practical experience with it, I think the order in which the BACKTRACE and the LOCATION fields are printed is wrong. I propose we put the LOCATION field before the BACKTRACE field, not after. This makes more sense because the location is effectively at the lowest level of the backtrace.

Makes sense, +1

cheers ./daniel

#3Alvaro Herrera
alvherre@2ndquadrant.com
In reply to: Daniel Gustafsson (#2)
Re: Log the location field before any backtrace

On 2020-Jul-09, Daniel Gustafsson wrote:

On 9 Jul 2020, at 11:17, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

In PG13, we added the ability to add backtraces to the log output. After some practical experience with it, I think the order in which the BACKTRACE and the LOCATION fields are printed is wrong. I propose we put the LOCATION field before the BACKTRACE field, not after. This makes more sense because the location is effectively at the lowest level of the backtrace.

Makes sense, +1

Likewise

--
�lvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

#4Michael Paquier
michael@paquier.xyz
In reply to: Alvaro Herrera (#3)
Re: Log the location field before any backtrace

On Thu, Jul 09, 2020 at 12:31:38PM -0400, Alvaro Herrera wrote:

On 2020-Jul-09, Daniel Gustafsson wrote:

On 9 Jul 2020, at 11:17, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

In PG13, we added the ability to add backtraces to the log
output. After some practical experience with it, I think the
order in which the BACKTRACE and the LOCATION fields are printed
is wrong. I propose we put the LOCATION field before the
BACKTRACE field, not after. This makes more sense because the
location is effectively at the lowest level of the backtrace.

Makes sense, +1

Likewise

+1.
--
Michael
#5Peter Eisentraut
peter.eisentraut@2ndquadrant.com
In reply to: Michael Paquier (#4)
Re: Log the location field before any backtrace

On 2020-07-10 04:04, Michael Paquier wrote:

On Thu, Jul 09, 2020 at 12:31:38PM -0400, Alvaro Herrera wrote:

On 2020-Jul-09, Daniel Gustafsson wrote:

On 9 Jul 2020, at 11:17, Peter Eisentraut <peter.eisentraut@2ndquadrant.com> wrote:

In PG13, we added the ability to add backtraces to the log
output. After some practical experience with it, I think the
order in which the BACKTRACE and the LOCATION fields are printed
is wrong. I propose we put the LOCATION field before the
BACKTRACE field, not after. This makes more sense because the
location is effectively at the lowest level of the backtrace.

Makes sense, +1

Likewise

+1.

committed

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services