something has gone wrong, but what is it?
Hi,
Today while hacking I encountered this delight:
2022-08-10 09:30:29.025 EDT [27126] FATAL: something has gone wrong
I actually already knew that something had gone wrong, because the
code I was writing was incomplete. And if I hadn't known that, the
word FATAL would have been a real good clue. What I was hoping was
that the error message might tell me WHAT had gone wrong, but it
didn't.
This seems to be the fault of Andres's commit
5aa4a9d2077fa902b4041245805082fec6be0648. In his defense, the addition
of any kind of elog() at that point in the code appears to be an
improvement over the previous state of affairs. Nonetheless I feel we
could do better still, as in the attached.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v1-0001-Be-more-specific-about-exactly-what-has-gone-wron.patchapplication/octet-stream; name=v1-0001-Be-more-specific-about-exactly-what-has-gone-wron.patchDownload
From 3fae2ce9fb0350f02142bec4ea60f52f48b9ca2f Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Wed, 10 Aug 2022 09:39:58 -0400
Subject: [PATCH v1] Be more specific about exactly what has gone wrong.
---
src/backend/postmaster/auxprocess.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 39ac4490db..28509a08f7 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -81,7 +81,7 @@ AuxiliaryProcessMain(AuxProcType auxtype)
MyBackendType = B_WAL_RECEIVER;
break;
default:
- elog(ERROR, "something has gone wrong");
+ elog(ERROR, "unrecognized AuxProcType: %d", (int) auxtype);
MyBackendType = B_INVALID;
}
--
2.24.3 (Apple Git-128)
On 10 Aug 2022, at 15:41, Robert Haas <robertmhaas@gmail.com> wrote:
I feel we could do better still, as in the attached.
+1, LGTM.
--
Daniel Gustafsson https://vmware.com/
Robert Haas <robertmhaas@gmail.com> writes:
- elog(ERROR, "something has gone wrong");
+ elog(ERROR, "unrecognized AuxProcType: %d", (int) auxtype);+1 ... the existing message is clearly not up to project standard.
regards, tom lane
On Wed, Aug 10, 2022 at 9:53 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
- elog(ERROR, "something has gone wrong"); + elog(ERROR, "unrecognized AuxProcType: %d", (int) auxtype);+1 ... the existing message is clearly not up to project standard.
After a bit of further looking around I noticed that there's another
check for an invalid auxtype in this function which uses a slightly
different message text and also PANIC rather than ERROR.
I think we should adopt that here too, for consistency, as in the attached.
The distinction between PANIC and ERROR doesn't really seem to matter
here. Either way, the server goes into an infinite crash-and-restart
loop. May as well be consistent.
--
Robert Haas
EDB: http://www.enterprisedb.com
Attachments:
v2-0001-Be-more-specific-about-exactly-what-has-gone-wron.patchapplication/octet-stream; name=v2-0001-Be-more-specific-about-exactly-what-has-gone-wron.patchDownload
From a8368568aadb067b5dee4a472b1adca4649d0703 Mon Sep 17 00:00:00 2001
From: Robert Haas <rhaas@postgresql.org>
Date: Wed, 10 Aug 2022 10:38:30 -0400
Subject: [PATCH v2] Be more specific about exactly what has gone wrong.
---
src/backend/postmaster/auxprocess.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/backend/postmaster/auxprocess.c b/src/backend/postmaster/auxprocess.c
index 39ac4490db..7765d1c83d 100644
--- a/src/backend/postmaster/auxprocess.c
+++ b/src/backend/postmaster/auxprocess.c
@@ -81,7 +81,7 @@ AuxiliaryProcessMain(AuxProcType auxtype)
MyBackendType = B_WAL_RECEIVER;
break;
default:
- elog(ERROR, "something has gone wrong");
+ elog(PANIC, "unrecognized process type: %d", (int) MyAuxProcType);
MyBackendType = B_INVALID;
}
--
2.24.3 (Apple Git-128)
Hi,
On 2022-08-10 10:49:59 -0400, Robert Haas wrote:
On Wed, Aug 10, 2022 at 9:53 AM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
- elog(ERROR, "something has gone wrong"); + elog(ERROR, "unrecognized AuxProcType: %d", (int) auxtype);+1 ... the existing message is clearly not up to project standard.
After a bit of further looking around I noticed that there's another
check for an invalid auxtype in this function which uses a slightly
different message text and also PANIC rather than ERROR.I think we should adopt that here too, for consistency, as in the attached.
The distinction between PANIC and ERROR doesn't really seem to matter
here. Either way, the server goes into an infinite crash-and-restart
loop. May as well be consistent.
Makes sense.
Greetings,
Andres Freund
On 10 Aug 2022, at 19:49, Robert Haas <robertmhaas@gmail.com> wrote:
After a bit of further looking around I noticed that there's another
check for an invalid auxtype in this function which uses a slightly
different message text and also PANIC rather than ERROR.
Is there a reason to do
MyBackendType = B_INVALID;
after PANIC or ERROR?
Best regards, Andrey Borodin.
On Wed, Aug 10, 2022 at 2:06 PM Andrey Borodin <x4mmm@yandex-team.ru> wrote:
On 10 Aug 2022, at 19:49, Robert Haas <robertmhaas@gmail.com> wrote:
After a bit of further looking around I noticed that there's another
check for an invalid auxtype in this function which uses a slightly
different message text and also PANIC rather than ERROR.Is there a reason to do
MyBackendType = B_INVALID;
after PANIC or ERROR?
That could probably be taken out, but it doesn't seem important to take it out.
--
Robert Haas
EDB: http://www.enterprisedb.com