public inbox for [email protected]help / color / mirror / Atom feed
[PATCH 8/8] Ignore correlation for new BRIN opclasses 6+ messages / 4 participants [nested] [flat]
* [PATCH 8/8] Ignore correlation for new BRIN opclasses @ 2020-09-12 13:07 Tomas Vondra <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Tomas Vondra @ 2020-09-12 13:07 UTC (permalink / raw) The new BRIN opclasses (bloom and minmax-multi) are less sensitive to poorly correlated data, so just assume the data is perfectly correlated during costing. Author: Tomas Vondra <[email protected]> Discussion: https://postgr.es/m/[email protected] --- src/backend/access/brin/brin_bloom.c | 1 + src/backend/access/brin/brin_minmax_multi.c | 1 + src/backend/utils/adt/selfuncs.c | 19 ++++++++++++++++++- src/include/access/brin_internal.h | 3 +++ 4 files changed, 23 insertions(+), 1 deletion(-) diff --git a/src/backend/access/brin/brin_bloom.c b/src/backend/access/brin/brin_bloom.c index c086e83236..22590b2351 100644 --- a/src/backend/access/brin/brin_bloom.c +++ b/src/backend/access/brin/brin_bloom.c @@ -412,6 +412,7 @@ brin_bloom_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(BloomOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (BloomOpaque *) diff --git a/src/backend/access/brin/brin_minmax_multi.c b/src/backend/access/brin/brin_minmax_multi.c index d2dc38adc8..6b1dd1040c 100644 --- a/src/backend/access/brin/brin_minmax_multi.c +++ b/src/backend/access/brin/brin_minmax_multi.c @@ -1737,6 +1737,7 @@ brin_minmax_multi_opcinfo(PG_FUNCTION_ARGS) result = palloc0(MAXALIGN(SizeofBrinOpcInfo(1)) + sizeof(MinmaxMultiOpaque)); + result->oi_ignore_correlation = true; result->oi_nstored = 1; result->oi_regular_nulls = true; result->oi_opaque = (MinmaxMultiOpaque *) diff --git a/src/backend/utils/adt/selfuncs.c b/src/backend/utils/adt/selfuncs.c index 52314d3aa1..0320d128f6 100644 --- a/src/backend/utils/adt/selfuncs.c +++ b/src/backend/utils/adt/selfuncs.c @@ -98,6 +98,7 @@ #include <math.h> #include "access/brin.h" +#include "access/brin_internal.h" #include "access/brin_page.h" #include "access/gin.h" #include "access/table.h" @@ -7352,7 +7353,8 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, double minimalRanges; double estimatedRanges; double selec; - Relation indexRel; + Relation indexRel = NULL; + TupleDesc tupdesc = NULL; ListCell *l; VariableStatData vardata; @@ -7374,6 +7376,7 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, */ indexRel = index_open(index->indexoid, NoLock); brinGetStats(indexRel, &statsData); + tupdesc = RelationGetDescr(indexRel); index_close(indexRel, NoLock); /* work out the actual number of ranges in the index */ @@ -7407,6 +7410,17 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, { IndexClause *iclause = lfirst_node(IndexClause, l); AttrNumber attnum = index->indexkeys[iclause->indexcol]; + FmgrInfo *opcInfoFn; + BrinOpcInfo *opcInfo; + Form_pg_attribute attr = TupleDescAttr(tupdesc, iclause->indexcol); + bool ignore_correlation; + + opcInfoFn = index_getprocinfo(indexRel, iclause->indexcol + 1, BRIN_PROCNUM_OPCINFO); + + opcInfo = (BrinOpcInfo *) + DatumGetPointer(FunctionCall1(opcInfoFn, attr->atttypid)); + + ignore_correlation = opcInfo->oi_ignore_correlation; /* attempt to lookup stats in relation for this index column */ if (attnum != 0) @@ -7477,6 +7491,9 @@ brincostestimate(PlannerInfo *root, IndexPath *path, double loop_count, if (sslot.nnumbers > 0) varCorrelation = Abs(sslot.numbers[0]); + if (ignore_correlation) + varCorrelation = 1.0; + if (varCorrelation > *indexCorrelation) *indexCorrelation = varCorrelation; diff --git a/src/include/access/brin_internal.h b/src/include/access/brin_internal.h index 89254b5766..063b703208 100644 --- a/src/include/access/brin_internal.h +++ b/src/include/access/brin_internal.h @@ -30,6 +30,9 @@ typedef struct BrinOpcInfo /* Regular processing of NULLs in BrinValues? */ bool oi_regular_nulls; + /* Ignore correlation during cost estimation */ + bool oi_ignore_correlation; + /* Opaque pointer for the opclass' private use */ void *oi_opaque; -- 2.26.2 --------------556A1DC61262AF15641DB204-- ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [HACKERS] psql casts aspersions on server reliability @ 2023-11-23 00:11 Bruce Momjian <[email protected]> 2023-11-23 00:38 ` Re: [HACKERS] psql casts aspersions on server reliability Tom Lane <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Bruce Momjian @ 2023-11-23 00:11 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers On Wed, Sep 28, 2016 at 09:14:41AM -0400, Tom Lane wrote: > Robert Haas <[email protected]> writes: > > psql tends to do things like this: > > rhaas=# select * from pg_stat_activity; > > FATAL: terminating connection due to administrator command > > server closed the connection unexpectedly > > This probably means the server terminated abnormally > > before or while processing the request. > > > Basically everything psql has to say about this is a lie: > > I cannot get terribly excited about this. What you seem to be proposing > is that psql try to intuit the reason for connection closure from the > last error message it got, but that seems likely to lead to worse lies > than printing a boilerplate message. > > I could go along with just dropping the last sentence ("This probably...") > if the last error we got was FATAL level. I don't find "unexpectedly" > to be problematic here: from the point of view of psql, and probably > of its user, the shutdown *was* unexpected. I looked at this thread from 2016 and I think the problem is the "abnormally" word, since if the server was shutdown by the administrator (most likely), it isn't abnormal. Here is a patch to remove "abnormally". -- Bruce Momjian <[email protected]> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. Attachments: [text/x-diff] exit.diff (2.7K, ../../[email protected]/2-exit.diff) download | inline diff: diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 660cdec93c..634708d716 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -749,7 +749,7 @@ retry4: */ definitelyEOF: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" + "\tThis probably means the server terminated\n" "\tbefore or while processing the request."); /* Come here if lower-level code already set a suitable errorMessage */ diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index f1192d28f2..115776ce6c 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -206,7 +206,7 @@ rloop: if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" + "\tThis probably means the server terminated\n" "\tbefore or while processing the request."); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", @@ -306,7 +306,7 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len) result_errno = SOCK_ERRNO; if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" + "\tThis probably means the server terminated\n" "\tbefore or while processing the request."); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index bd72a87bbb..b972bd3ced 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -233,7 +233,7 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len) case EPIPE: case ECONNRESET: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" + "\tThis probably means the server terminated\n" "\tbefore or while processing the request."); break; @@ -395,7 +395,7 @@ retry_masked: /* (strdup failure is OK, we'll cope later) */ snprintf(msgbuf, sizeof(msgbuf), libpq_gettext("server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" + "\tThis probably means the server terminated\n" "\tbefore or while processing the request.")); /* keep newline out of translated string */ strlcat(msgbuf, "\n", sizeof(msgbuf)); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [HACKERS] psql casts aspersions on server reliability 2023-11-23 00:11 Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> @ 2023-11-23 00:38 ` Tom Lane <[email protected]> 2023-11-23 03:25 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Tom Lane @ 2023-11-23 00:38 UTC (permalink / raw) To: Bruce Momjian <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers Bruce Momjian <[email protected]> writes: > On Wed, Sep 28, 2016 at 09:14:41AM -0400, Tom Lane wrote: >> I could go along with just dropping the last sentence ("This probably...") >> if the last error we got was FATAL level. I don't find "unexpectedly" >> to be problematic here: from the point of view of psql, and probably >> of its user, the shutdown *was* unexpected. > I looked at this thread from 2016 and I think the problem is the > "abnormally" word, since if the server was shutdown by the administrator > (most likely), it isn't abnormal. Here is a patch to remove > "abnormally". I do not think this is an improvement. The places you are changing are reacting to a connection closure. *If* we had previously gotten a "FATAL: terminating connection due to administrator command" message, then yeah the connection closure is expected; but if not, it isn't. Your patch adds no check for that. (As I remarked in 2016, we could probably condition this on the elevel being FATAL, rather than checking for specific error messages.) regards, tom lane ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [HACKERS] psql casts aspersions on server reliability 2023-11-23 00:11 Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 00:38 ` Re: [HACKERS] psql casts aspersions on server reliability Tom Lane <[email protected]> @ 2023-11-23 03:25 ` Bruce Momjian <[email protected]> 2023-11-23 16:12 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Bruce Momjian @ 2023-11-23 03:25 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers On Wed, Nov 22, 2023 at 07:38:52PM -0500, Tom Lane wrote: > Bruce Momjian <[email protected]> writes: > > On Wed, Sep 28, 2016 at 09:14:41AM -0400, Tom Lane wrote: > >> I could go along with just dropping the last sentence ("This probably...") > >> if the last error we got was FATAL level. I don't find "unexpectedly" > >> to be problematic here: from the point of view of psql, and probably > >> of its user, the shutdown *was* unexpected. > > > I looked at this thread from 2016 and I think the problem is the > > "abnormally" word, since if the server was shutdown by the administrator > > (most likely), it isn't abnormal. Here is a patch to remove > > "abnormally". > > I do not think this is an improvement. The places you are changing > are reacting to a connection closure. *If* we had previously gotten a > "FATAL: terminating connection due to administrator command" message, > then yeah the connection closure is expected; but if not, it isn't. > Your patch adds no check for that. (As I remarked in 2016, we could > probably condition this on the elevel being FATAL, rather than > checking for specific error messages.) Yes, you are correct. Here is a patch that implements the FATAL test, though I am not sure I have the logic correct or backwards, and I don't know how to test this. Thanks. -- Bruce Momjian <[email protected]> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. Attachments: [text/x-diff] exit.diff (3.6K, ../../[email protected]/2-exit.diff) download | inline diff: diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 660cdec93c..c541fd8b02 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -749,8 +749,10 @@ retry4: */ definitelyEOF: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + conn->result->resultStatus == PGRES_FATAL_ERROR ? + "" : "abnormally"); /* Come here if lower-level code already set a suitable errorMessage */ definitelyFailed: diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index f1192d28f2..6c21f91817 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -206,8 +206,10 @@ rloop: if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + conn->result->resultStatus == PGRES_FATAL_ERROR ? + "" : "abnormally"); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", SOCK_STRERROR(result_errno, @@ -306,8 +308,10 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len) result_errno = SOCK_ERRNO; if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + conn->result->resultStatus == PGRES_FATAL_ERROR ? + "" : "abnormally"); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", SOCK_STRERROR(result_errno, diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index bd72a87bbb..5e7136195a 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -233,8 +233,10 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len) case EPIPE: case ECONNRESET: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + conn->result->resultStatus == PGRES_FATAL_ERROR ? + "" : "abnormally"); break; default: @@ -395,8 +397,11 @@ retry_masked: /* (strdup failure is OK, we'll cope later) */ snprintf(msgbuf, sizeof(msgbuf), libpq_gettext("server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request.")); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request."), + conn->result->resultStatus == PGRES_FATAL_ERROR ? + "" : "abnormally"); + /* keep newline out of translated string */ strlcat(msgbuf, "\n", sizeof(msgbuf)); conn->write_err_msg = strdup(msgbuf); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [HACKERS] psql casts aspersions on server reliability 2023-11-23 00:11 Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 00:38 ` Re: [HACKERS] psql casts aspersions on server reliability Tom Lane <[email protected]> 2023-11-23 03:25 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> @ 2023-11-23 16:12 ` Bruce Momjian <[email protected]> 2023-11-24 03:06 ` Re: [HACKERS] psql casts aspersions on server reliability Laurenz Albe <[email protected]> 0 siblings, 1 reply; 6+ messages in thread From: Bruce Momjian @ 2023-11-23 16:12 UTC (permalink / raw) To: Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers On Wed, Nov 22, 2023 at 10:25:14PM -0500, Bruce Momjian wrote: > On Wed, Nov 22, 2023 at 07:38:52PM -0500, Tom Lane wrote: > > Bruce Momjian <[email protected]> writes: > > > On Wed, Sep 28, 2016 at 09:14:41AM -0400, Tom Lane wrote: > > >> I could go along with just dropping the last sentence ("This probably...") > > >> if the last error we got was FATAL level. I don't find "unexpectedly" > > >> to be problematic here: from the point of view of psql, and probably > > >> of its user, the shutdown *was* unexpected. > > > > > I looked at this thread from 2016 and I think the problem is the > > > "abnormally" word, since if the server was shutdown by the administrator > > > (most likely), it isn't abnormal. Here is a patch to remove > > > "abnormally". > > > > I do not think this is an improvement. The places you are changing > > are reacting to a connection closure. *If* we had previously gotten a > > "FATAL: terminating connection due to administrator command" message, > > then yeah the connection closure is expected; but if not, it isn't. > > Your patch adds no check for that. (As I remarked in 2016, we could > > probably condition this on the elevel being FATAL, rather than > > checking for specific error messages.) > > Yes, you are correct. Here is a patch that implements the FATAL test, > though I am not sure I have the logic correct or backwards, and I don't > know how to test this. Thanks. I developed the attached patch which seems to work better. In testing kill -3 on a backend or calling elog(FATAL) in the server for a session, libpq's 'res' is NULL, meaning we don't have any status to check for PGRES_FATAL_ERROR. It is very possible that libpq just isn't stuctured to have the PGRES_FATAL_ERROR at the point where we issue this message, and this is not worth improving. test=> select pg_sleep(100); --> FATAL: FATAL called server closed the connection unexpectedly --> This probably means the server terminated null before or while processing the request. The connection to the server was lost. Attempting reset: Succeeded. -- Bruce Momjian <[email protected]> https://momjian.us EDB https://enterprisedb.com Only you can decide what is important to you. Attachments: [text/x-diff] exit.diff (4.4K, ../../[email protected]/2-exit.diff) download | inline diff: diff --git a/src/interfaces/libpq/fe-misc.c b/src/interfaces/libpq/fe-misc.c index 660cdec93c..64faad19df 100644 --- a/src/interfaces/libpq/fe-misc.c +++ b/src/interfaces/libpq/fe-misc.c @@ -749,8 +749,11 @@ retry4: */ definitelyEOF: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + (conn->result == NULL) ? " null" : + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? + "" : " abnormally"); /* Come here if lower-level code already set a suitable errorMessage */ definitelyFailed: diff --git a/src/interfaces/libpq/fe-protocol3.c b/src/interfaces/libpq/fe-protocol3.c index 5613c56b14..03914b97fc 100644 --- a/src/interfaces/libpq/fe-protocol3.c +++ b/src/interfaces/libpq/fe-protocol3.c @@ -2158,6 +2158,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid, if (pqGetErrorNotice3(conn, true)) continue; status = PGRES_FATAL_ERROR; + fprintf(stderr, "Got 'E'\n"); break; case 'A': /* notify message */ /* handle notify and go back to processing return values */ diff --git a/src/interfaces/libpq/fe-secure-openssl.c b/src/interfaces/libpq/fe-secure-openssl.c index f1192d28f2..f4c7f51b0a 100644 --- a/src/interfaces/libpq/fe-secure-openssl.c +++ b/src/interfaces/libpq/fe-secure-openssl.c @@ -206,8 +206,11 @@ rloop: if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + (conn->result == NULL) ? " null" : + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? + "" : " abnormally"); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", SOCK_STRERROR(result_errno, @@ -306,8 +309,11 @@ pgtls_write(PGconn *conn, const void *ptr, size_t len) result_errno = SOCK_ERRNO; if (result_errno == EPIPE || result_errno == ECONNRESET) libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + (conn->result == NULL) ? " null" : + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? + "" : " abnormally"); else libpq_append_conn_error(conn, "SSL SYSCALL error: %s", SOCK_STRERROR(result_errno, diff --git a/src/interfaces/libpq/fe-secure.c b/src/interfaces/libpq/fe-secure.c index bd72a87bbb..be93c2c0f9 100644 --- a/src/interfaces/libpq/fe-secure.c +++ b/src/interfaces/libpq/fe-secure.c @@ -233,8 +233,11 @@ pqsecure_raw_read(PGconn *conn, void *ptr, size_t len) case EPIPE: case ECONNRESET: libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request."); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request.", + (conn->result == NULL) ? " null" : + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? + "" : " abnormally"); break; default: @@ -395,8 +398,12 @@ retry_masked: /* (strdup failure is OK, we'll cope later) */ snprintf(msgbuf, sizeof(msgbuf), libpq_gettext("server closed the connection unexpectedly\n" - "\tThis probably means the server terminated abnormally\n" - "\tbefore or while processing the request.")); + "\tThis probably means the server terminated%s\n" + "\tbefore or while processing the request."), + (conn->result == NULL) ? " null" : + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? + "" : " abnormally"); + /* keep newline out of translated string */ strlcat(msgbuf, "\n", sizeof(msgbuf)); conn->write_err_msg = strdup(msgbuf); ^ permalink raw reply [nested|flat] 6+ messages in thread
* Re: [HACKERS] psql casts aspersions on server reliability 2023-11-23 00:11 Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 00:38 ` Re: [HACKERS] psql casts aspersions on server reliability Tom Lane <[email protected]> 2023-11-23 03:25 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 16:12 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> @ 2023-11-24 03:06 ` Laurenz Albe <[email protected]> 0 siblings, 0 replies; 6+ messages in thread From: Laurenz Albe @ 2023-11-24 03:06 UTC (permalink / raw) To: Bruce Momjian <[email protected]>; Tom Lane <[email protected]>; +Cc: Robert Haas <[email protected]>; pgsql-hackers On Thu, 2023-11-23 at 11:12 -0500, Bruce Momjian wrote: > On Wed, Nov 22, 2023 at 10:25:14PM -0500, Bruce Momjian wrote: > > Yes, you are correct. Here is a patch that implements the FATAL test, > > though I am not sure I have the logic correct or backwards, and I don't > > know how to test this. Thanks. > > I developed the attached patch which seems to work better. In testing > kill -3 on a backend or calling elog(FATAL) in the server for a > session, libpq's 'res' is NULL, meaning we don't have any status to > check for PGRES_FATAL_ERROR. It is very possible that libpq just isn't > stuctured to have the PGRES_FATAL_ERROR at the point where we issue this > message, and this is not worth improving. > > test=> select pg_sleep(100); > --> FATAL: FATAL called > > server closed the connection unexpectedly > --> This probably means the server terminated null > before or while processing the request. > The connection to the server was lost. Attempting reset: Succeeded. I don't thing "terminated null" is a meaningful message. > libpq_append_conn_error(conn, "server closed the connection unexpectedly\n" > - "\tThis probably means the server terminated abnormally\n" > - "\tbefore or while processing the request."); > + "\tThis probably means the server terminated%s\n" > + "\tbefore or while processing the request.", > + (conn->result == NULL) ? " null" : > + (conn->result->resultStatus == PGRES_FATAL_ERROR) ? > + "" : " abnormally"); Apart from the weird "null", will that work well for translation? > --- a/src/interfaces/libpq/fe-protocol3.c > +++ b/src/interfaces/libpq/fe-protocol3.c > @@ -2158,6 +2158,7 @@ pqFunctionCall3(PGconn *conn, Oid fnid, > if (pqGetErrorNotice3(conn, true)) > continue; > status = PGRES_FATAL_ERROR; > + fprintf(stderr, "Got 'E'\n"); > break; > case 'A': /* notify message */ > /* handle notify and go back to processing return values */ That looks like a leftover debugging message. Yours, Laurenz Albe ^ permalink raw reply [nested|flat] 6+ messages in thread
end of thread, other threads:[~2023-11-24 03:06 UTC | newest] Thread overview: 6+ messages (download: mbox mbox.gz follow: Atom feed) -- links below jump to the message on this page -- 2020-09-12 13:07 [PATCH 8/8] Ignore correlation for new BRIN opclasses Tomas Vondra <[email protected]> 2023-11-23 00:11 Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 00:38 ` Re: [HACKERS] psql casts aspersions on server reliability Tom Lane <[email protected]> 2023-11-23 03:25 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-23 16:12 ` Re: [HACKERS] psql casts aspersions on server reliability Bruce Momjian <[email protected]> 2023-11-24 03:06 ` Re: [HACKERS] psql casts aspersions on server reliability Laurenz Albe <[email protected]>
This inbox is served by agora; see mirroring instructions for how to clone and mirror all data and code used for this inbox