Thread: 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
Attachment
> 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
Attachment
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