pg.ddx.io pgsql-hackers@postgresql.org mailing list archive help / color / mirror / Atom feed From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Robert Haas <robertmhaas@gmail.com>
Cc: David Rowley <david.rowley@2ndquadrant.com>
Cc: Amit Langote <amitlangote09@gmail.com>
Cc: Amit Langote <Langote_Amit_f8@lab.ntt.co.jp>
Cc: Jesper Pedersen <jesper.pedersen@redhat.com>
Cc: Rajkumar Raghuwanshi <rajkumar.raghuwanshi@enterprisedb.com>
Cc: Beena Emerson <memissemerson@gmail.com>
Cc: amul sul <sulamul@gmail.com>
Cc: pgsql-hackers@postgresql.org <pgsql-hackers@postgresql.org>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Subject: Re: [HACKERS] Runtime Partition Pruning
Date: Tue, 10 Apr 2018 18:32:03 -0300
Message-ID: <20180410213203.nl645o5vj5ap66sl@alvherre.pgsql> (raw )
In-Reply-To: <CA+TgmoY64G_VRyRgAfB3x9RdNJ=oozmSaj1d6_Oi9Ab52d+OHg@mail.gmail.com >
Robert Haas wrote:
> On Mon, Apr 9, 2018 at 2:28 PM, Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
> >> I don't get this. The executor surely had to (and did) open all of
> >> the relations somewhere even before this patch.
> > I was worried that this coding could be seen as breaking modularity, or
> > trying to do excessive work. However, after looking closer at it, it
> > doesn't really look like it's the case. So, nevermind.
>
> Well what I'm saying is that it shouldn't be necessary. If the
> relations are being opened already and the pointers to the relcache
> entries are being saved someplace, you shouldn't need to re-open them
> elsewhere to get pointers to the relcache entries.
I looked a bit more into this. It turns out that we have indeed opened
the relation before -- first in parserOpenTable (for addRangeTableEntry),
then in expandRTE, then in QueryRewrite, then in subquery_planner, then
in get_relation_info.
So, frankly, since each module thinks it's okay to open it every once in
a while, I'm not sure we should be terribly stressed about doing it once
more for partition pruning. Particularly since communicating the
pointer seems to be quite troublesome.
To figure out, I used the attached patch (not intended for application)
to add a backtrace to each log message, plus a couple of accusatory
elog() calls in relation_open and ExecSetupPartitionPruneState.
--
Álvaro Herrera https://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 5d682c8cb31f79c79c2ba4cafeb876f10b072cfc Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 10 Apr 2018 18:30:29 -0300
Subject: [PATCH] errbacktrace
---
src/backend/utils/error/elog.c | 59 +++++++++++++++++++++++++++++++++++++
src/include/postgres_ext.h | 1 +
src/include/utils/elog.h | 3 ++
src/interfaces/libpq/fe-protocol3.c | 3 ++
4 files changed, 66 insertions(+)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 16531f7a0f..e531d0009e 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,7 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#include <execinfo.h>
#include "access/transam.h"
#include "access/xact.h"
@@ -493,6 +494,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -811,6 +814,41 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+#define BACKTRACE_FRAMES 100
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+ void *buf[BACKTRACE_FRAMES];
+ int nframes;
+ char **strfrms;
+ StringInfoData errtrace;
+ int i;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ nframes = backtrace(buf, BACKTRACE_FRAMES);
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return 0;
+
+ initStringInfo(&errtrace);
+
+ /* the first frame is always errbacktrace itself, so skip it */
+ for (i = 1; i < nframes; i++)
+ appendStringInfo(&errtrace, "%s\n", strfrms[i]);
+ free(strfrms);
+
+ edata->backtrace = errtrace.data;
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1522,6 +1560,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1560,6 +1600,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1635,6 +1677,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1702,6 +1746,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2932,6 +2978,13 @@ 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... */
@@ -3209,6 +3262,12 @@ send_message_to_frontend(ErrorData *edata)
err_sendstring(&msgbuf, edata->context);
}
+ if (edata->backtrace)
+ {
+ pq_sendbyte(&msgbuf, PG_DIAG_BACKTRACE);
+ err_sendstring(&msgbuf, edata->backtrace);
+ }
+
if (edata->schema_name)
{
pq_sendbyte(&msgbuf, PG_DIAG_SCHEMA_NAME);
diff --git a/src/include/postgres_ext.h b/src/include/postgres_ext.h
index fdb61b7cf5..799f0009f0 100644
--- a/src/include/postgres_ext.h
+++ b/src/include/postgres_ext.h
@@ -62,6 +62,7 @@ typedef PG_INT64_TYPE pg_int64;
#define PG_DIAG_INTERNAL_POSITION 'p'
#define PG_DIAG_INTERNAL_QUERY 'q'
#define PG_DIAG_CONTEXT 'W'
+#define PG_DIAG_BACKTRACE 'B'
#define PG_DIAG_SCHEMA_NAME 's'
#define PG_DIAG_TABLE_NAME 't'
#define PG_DIAG_COLUMN_NAME 'c'
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7a9ba7f2ff..a4343794b0 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -170,6 +170,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -345,6 +347,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index d3ca5d25f6..3d9e40a09c 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1079,6 +1079,9 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
appendPQExpBuffer(msg, libpq_gettext("CONTEXT: %s\n"),
val);
}
+ val = PQresultErrorField(res, PG_DIAG_BACKTRACE);
+ if (val)
+ appendPQExpBuffer(msg, libpq_gettext("BACKTRACE: %s\n"), val);
}
if (verbosity == PQERRORS_VERBOSE)
{
--
2.11.0
Attachments:
0001-errbacktrace.patch (text/plain, 5.9K) download
From 5d682c8cb31f79c79c2ba4cafeb876f10b072cfc Mon Sep 17 00:00:00 2001
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
Date: Tue, 10 Apr 2018 18:30:29 -0300
Subject: [PATCH] errbacktrace
---
src/backend/utils/error/elog.c | 59 +++++++++++++++++++++++++++++++++++++
src/include/postgres_ext.h | 1 +
src/include/utils/elog.h | 3 ++
src/interfaces/libpq/fe-protocol3.c | 3 ++
4 files changed, 66 insertions(+)
diff --git a/src/backend/utils/error/elog.c b/src/backend/utils/error/elog.c
index 16531f7a0f..e531d0009e 100644
--- a/src/backend/utils/error/elog.c
+++ b/src/backend/utils/error/elog.c
@@ -62,6 +62,7 @@
#ifdef HAVE_SYSLOG
#include <syslog.h>
#endif
+#include <execinfo.h>
#include "access/transam.h"
#include "access/xact.h"
@@ -493,6 +494,8 @@ errfinish(int dummy,...)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -811,6 +814,41 @@ errmsg(const char *fmt,...)
return 0; /* return value does not matter */
}
+#define BACKTRACE_FRAMES 100
+int
+errbacktrace(void)
+{
+ ErrorData *edata = &errordata[errordata_stack_depth];
+ MemoryContext oldcontext;
+ void *buf[BACKTRACE_FRAMES];
+ int nframes;
+ char **strfrms;
+ StringInfoData errtrace;
+ int i;
+
+ recursion_depth++;
+ CHECK_STACK_DEPTH();
+ oldcontext = MemoryContextSwitchTo(edata->assoc_context);
+
+ nframes = backtrace(buf, BACKTRACE_FRAMES);
+ strfrms = backtrace_symbols(buf, nframes);
+ if (strfrms == NULL)
+ return 0;
+
+ initStringInfo(&errtrace);
+
+ /* the first frame is always errbacktrace itself, so skip it */
+ for (i = 1; i < nframes; i++)
+ appendStringInfo(&errtrace, "%s\n", strfrms[i]);
+ free(strfrms);
+
+ edata->backtrace = errtrace.data;
+
+ MemoryContextSwitchTo(oldcontext);
+ recursion_depth--;
+
+ return 0;
+}
/*
* errmsg_internal --- add a primary error message text to the current error
@@ -1522,6 +1560,8 @@ CopyErrorData(void)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -1560,6 +1600,8 @@ FreeErrorData(ErrorData *edata)
pfree(edata->hint);
if (edata->context)
pfree(edata->context);
+ if (edata->backtrace)
+ pfree(edata->backtrace);
if (edata->schema_name)
pfree(edata->schema_name);
if (edata->table_name)
@@ -1635,6 +1677,8 @@ ThrowErrorData(ErrorData *edata)
newedata->hint = pstrdup(edata->hint);
if (edata->context)
newedata->context = pstrdup(edata->context);
+ if (edata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
/* assume message_id is not available */
if (edata->schema_name)
newedata->schema_name = pstrdup(edata->schema_name);
@@ -1702,6 +1746,8 @@ ReThrowError(ErrorData *edata)
newedata->hint = pstrdup(newedata->hint);
if (newedata->context)
newedata->context = pstrdup(newedata->context);
+ if (newedata->backtrace)
+ newedata->backtrace = pstrdup(newedata->backtrace);
if (newedata->schema_name)
newedata->schema_name = pstrdup(newedata->schema_name);
if (newedata->table_name)
@@ -2932,6 +2978,13 @@ 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... */
@@ -3209,6 +3262,12 @@ send_message_to_frontend(ErrorData *edata)
err_sendstring(&msgbuf, edata->context);
}
+ if (edata->backtrace)
+ {
+ pq_sendbyte(&msgbuf, PG_DIAG_BACKTRACE);
+ err_sendstring(&msgbuf, edata->backtrace);
+ }
+
if (edata->schema_name)
{
pq_sendbyte(&msgbuf, PG_DIAG_SCHEMA_NAME);
diff --git a/src/include/postgres_ext.h b/src/include/postgres_ext.h
index fdb61b7cf5..799f0009f0 100644
--- a/src/include/postgres_ext.h
+++ b/src/include/postgres_ext.h
@@ -62,6 +62,7 @@ typedef PG_INT64_TYPE pg_int64;
#define PG_DIAG_INTERNAL_POSITION 'p'
#define PG_DIAG_INTERNAL_QUERY 'q'
#define PG_DIAG_CONTEXT 'W'
+#define PG_DIAG_BACKTRACE 'B'
#define PG_DIAG_SCHEMA_NAME 's'
#define PG_DIAG_TABLE_NAME 't'
#define PG_DIAG_COLUMN_NAME 'c'
diff --git a/src/include/utils/elog.h b/src/include/utils/elog.h
index 7a9ba7f2ff..a4343794b0 100644
--- a/src/include/utils/elog.h
+++ b/src/include/utils/elog.h
@@ -170,6 +170,8 @@ extern int errcontext_msg(const char *fmt,...) pg_attribute_printf(1, 2);
extern int errhidestmt(bool hide_stmt);
extern int errhidecontext(bool hide_ctx);
+extern int errbacktrace(void);
+
extern int errfunction(const char *funcname);
extern int errposition(int cursorpos);
@@ -345,6 +347,7 @@ typedef struct ErrorData
char *detail_log; /* detail error message for server log only */
char *hint; /* hint message */
char *context; /* context message */
+ char *backtrace; /* backtrace */
const char *message_id; /* primary message's id (original string) */
char *schema_name; /* name of schema */
char *table_name; /* name of table */
diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c
index d3ca5d25f6..3d9e40a09c 100644
--- a/src/interfaces/libpq/fe-protocol3.c
+++ b/src/interfaces/libpq/fe-protocol3.c
@@ -1079,6 +1079,9 @@ pqBuildErrorMessage3(PQExpBuffer msg, const PGresult *res,
appendPQExpBuffer(msg, libpq_gettext("CONTEXT: %s\n"),
val);
}
+ val = PQresultErrorField(res, PG_DIAG_BACKTRACE);
+ if (val)
+ appendPQExpBuffer(msg, libpq_gettext("BACKTRACE: %s\n"), val);
}
if (verbosity == PQERRORS_VERBOSE)
{
--
2.11.0
view thread (186+ messages) latest in thread
Message-ID: <20180410213203.nl645o5vj5ap66sl@alvherre.pgsql>
Permalink: ../20180410213203.nl645o5vj5ap66sl@alvherre.pgsql/
Also on: postgresql.org/message-id/20180410213203.nl645o5vj5ap66sl@alvherre.pgsql
copy link · copy postgr.es · copy git am
reply Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-hackers@postgresql.org
Cc: alvherre@alvh.no-ip.org, robertmhaas@gmail.com, david.rowley@2ndquadrant.com, amitlangote09@gmail.com, Langote_Amit_f8@lab.ntt.co.jp, jesper.pedersen@redhat.com, rajkumar.raghuwanshi@enterprisedb.com, memissemerson@gmail.com, sulamul@gmail.com, dilipbalaut@gmail.com
Subject: Re: [HACKERS] Runtime Partition Pruning
In-Reply-To: <20180410213203.nl645o5vj5ap66sl@alvherre.pgsql>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox