Thread: elog() proposal
I just submitted a patch to fix various elog() issues. I have two additional proposals. First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause compile problems/warnings, especially with Perl. I suggest renaming all elog levels to PG*, so it would be PGERROR and PGINFO. We could also do E_* or E*. I am interested in other opinions. Second, I propose adding two GUC variables that control how much elog() info is sent to the server and client logs. I suggest 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR, FATAL, CRASH; and 'client_message_min' with possible values INFO, NOTICE, ERROR, FATAL, CRASH. We currently have 'debug_level' which controls how much debug information is sent to the server logs. I believe this would have to remain because it controls how _much_ DEBUG output is printed. We could go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum reporting mode with level 5. This functionality mimics the log filter functionality of syslog(3). Comments? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
On Wed, Feb 20, 2002 at 09:54:51PM -0500, Bruce Momjian wrote:
> I just submitted a patch to fix various elog() issues. I have two
> additional proposals.
>
> First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
> compile problems/warnings, especially with Perl. I suggest renaming all
> elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
> E_* or E*. I am interested in other opinions.
You forgot PG_* :-)
> Second, I propose adding two GUC variables that control how much elog()
> info is sent to the server and client logs. I suggest
> 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
> FATAL, CRASH; and 'client_message_min' with possible values INFO,
> NOTICE, ERROR, FATAL, CRASH.
IMHO stop an example NOTICE by "SET NOTICE TO 'OFF'" is better and more common way than current the only one way in
libpqand PQsetNoticeProcessor.
Karel
-- Karel Zak <zakkr@zf.jcu.cz>http://home.zf.jcu.cz/~zakkr/C, PostgreSQL, PHP, WWW, http://docs.linux.cz,
http://mape.jcu.cz
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
> compile problems/warnings, especially with Perl. I suggest renaming all
> elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
> E_* or E*. I am interested in other opinions.
We must put a prefix on these things. I have no strong opinion about
which prefix to use, but the change is long overdue.
> Second, I propose adding two GUC variables that control how much elog()
> info is sent to the server and client logs. I suggest
> 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
> FATAL, CRASH; and 'client_message_min' with possible values INFO,
> NOTICE, ERROR, FATAL, CRASH.
client_message_min cannot usefully be set higher than ERROR. It will
certainly break libpq, for example, if 'E' messages don't come back when
a command fails. I also object to the idea that client_message_min
should not be settable to a level that allows DEBUG messages to be seen.
That would frequently save having to scrounge in the postmaster log,
which is not that easy if you're a client on a different machine.
> We currently have 'debug_level' which controls how much debug
> information is sent to the server logs. I believe this would have to
> remain because it controls how _much_ DEBUG output is printed. We could
> go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum
> reporting mode with level 5.
I think you missed the point of my previous suggestion: let's invent
multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4,
and replace code like
if (DebugLevel > 4) elog(DEBUG, ...);
with
elog(PGDEBUG4, ...);
This way, the log verbosity is controllable by a single mechanism rather
than two independent variables. You can set server_message_min to, say,
DEBUG or DEBUG4.
Finally, I still object strongly to the notion that any of these message
levels should be hard wired as "never send to client" or "never send to
log"; if you can't imagine scenarios where people will want to override
that behavior, then your imagination is lacking. I want that choice to
be completely configurable. What I'd suggest is an exclusion mask.
Suppose we order the levels as
PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR,
PGFATAL, PGCRASH.
Then I think from the client's point of view the only knob needed is
client_message_min (settable between PGDEBUG4 and PGERROR, with default
PGINFO). On the server side we could have server_message_min (settable
between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a
list server_message_exclude of levels not to log, which could have
default value "PGINFO,PGNOTICE" (internally this would become a bitmask
so it would be cheap to test).
It'd be possible to unify server_message_min and server_message_exclude
into a single parameter that's just a message level bitmask, but I think
the two separate parameters would be easier to deal with; otherwise you
end up with an awfully verbose parameter value.
regards, tom lane
Bruce Momjian writes: > First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause > compile problems/warnings, especially with Perl. I suggest renaming all > elog levels to PG*, so it would be PGERROR and PGINFO. We could also do > E_* or E*. I am interested in other opinions. I think there should be separately named functions. That would relieve us from inserting many dummy return statements because it gives the compiler a clue of what's going on, and we don't have to tag all strings as translatable. > Second, I propose adding two GUC variables that control how much elog() > info is sent to the server and client logs. I suggest > 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR, > FATAL, CRASH; and 'client_message_min' with possible values INFO, > NOTICE, ERROR, FATAL, CRASH. I think there are a lot more possible combinations of behaviour we need to take care of and we might as well get rid of this linear level system altogether. Some categorizations: * How to proceed: return to caller, abort transaction, abort session, abort all sessions * Where to send output: client, server log, both * Type of error: code bug (assert), server failure, user/client error Furthermore, in some instances you may want to send a different message to client and server. (E.g., a client doesn't necessarily have the right to know the exact cause of a server crash.) Also, user errors need to be able to carry information such as error codes. So, what we need to do is to figure out exactly which of the above combinations are useful or desirable, what parameters they take, etc. I don't think just renaming a couple of things makes things better. Also, there needs to be a transition plan, or suddenly every user-compiled module is broken. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> I think there should be separately named functions.
You mean elog_debug(), elog_error(), ...? Seems pretty yucky.
> That would relieve us
> from inserting many dummy return statements because it gives the compiler
> a clue of what's going on,
Only in gcc; if we remove the return statements then we still will get
warnings in compilers that don't recognize gcc's attributes. I think
this is a *bad* idea.
> and we don't have to tag all strings as translatable.
Huh? How does having multiple functions instead of one help there?
> I think there are a lot more possible combinations of behaviour we need to
> take care of and we might as well get rid of this linear level system
> altogether. Some categorizations:
> * How to proceed: return to caller, abort transaction, abort session,
> abort all sessions
> * Where to send output: client, server log, both
> * Type of error: code bug (assert), server failure, user/client error
Dubious. "How to proceed" clearly must be encoded in the elog call,
since generally the code surrounding the call point is not prepared to
cope with alternative behaviors. However, "where to send output" ought
to be configurable, as I have argued elsewhere. I'm not sure that "type
of error" really conveys much, or that we can always tell what the cause
of an error report is.
> Furthermore, in some instances you may want to send a different message to
> client and server.
Hmm, maybe, but I sure don't want to be passing two strings in every
elog call. There might be a *small* number of cases where the above is
true, and we could use a specialized variant of elog for them.
> Also, user errors need to be able to carry information such as error
> codes.
Agreed, but I suggested a grand revision of elog calls a year ago, and
no one had the energy to take it on. I think we shall have to resign
ourselves to fixing one issue at a time... unless you want to go back
to the grand plan?
> there needs to be a transition plan, or suddenly every user-compiled
> module is broken.
Also agreed. Probably the new function has to be called something else
than elog in any case, so that we can have a compatibility layer to
accept old elog calls. (That'd avoid requiring a "big bang" patch, too,
which'd surely ease making the changes.)
regards, tom lane
OK, let me throw out a modified proposal that attempts to deal with these issues, and avoids creating a complicated error reporting system that few will understand or be able to control. How about if we have a new *_min_message setting, INFOLOG, which grabs client and server log info. This will allow a unified system where the client_min_message can be set to: DEBUG, INFOLOG, INFO, NOTICE, ERROR default, INFO, and the server_min_message can be: DEBUG, INFOLOG, LOG, NOTICE, ERROR, FATAL, CRASH default, LOG. Basically we do this: elog(INFO, ...) and this prints to the client if its value is INFO or less, and prints to the server if its level is INFOLOG or less. In this case: elog(LOG, ...) it prints to the client if its value is INFOLOG or less, and prints to the server if its level is LOG or less. This allows elog() to go to both places, if desired, and allows control in a fine-grained manner. This handles Tom's issue that DEBUG should be visible to the client if it wishes. Peter's issue of sending separate messages to client and server can be handled with two elog messages, one LOG, another INFO. This is all backward compatible because it does not remove any codes, only adds elog levels INFO and LOG, which are lower than NOTICE. Also, I like Tom's idea of DEBUG4 in the code, rather than testing the debug level. Instead, do the level testing in elog() function, which makes sense. This does allow DEBUGx as a possible debug level. --------------------------------------------------------------------------- Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause > > compile problems/warnings, especially with Perl. I suggest renaming all > > elog levels to PG*, so it would be PGERROR and PGINFO. We could also do > > E_* or E*. I am interested in other opinions. > > We must put a prefix on these things. I have no strong opinion about > which prefix to use, but the change is long overdue. > > > Second, I propose adding two GUC variables that control how much elog() > > info is sent to the server and client logs. I suggest > > 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR, > > FATAL, CRASH; and 'client_message_min' with possible values INFO, > > NOTICE, ERROR, FATAL, CRASH. > > client_message_min cannot usefully be set higher than ERROR. It will > certainly break libpq, for example, if 'E' messages don't come back when > a command fails. I also object to the idea that client_message_min > should not be settable to a level that allows DEBUG messages to be seen. > That would frequently save having to scrounge in the postmaster log, > which is not that easy if you're a client on a different machine. > > > We currently have 'debug_level' which controls how much debug > > information is sent to the server logs. I believe this would have to > > remain because it controls how _much_ DEBUG output is printed. We could > > go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum > > reporting mode with level 5. > > I think you missed the point of my previous suggestion: let's invent > multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4, > and replace code like > > if (DebugLevel > 4) > elog(DEBUG, ...); > > with > > elog(PGDEBUG4, ...); > > This way, the log verbosity is controllable by a single mechanism rather > than two independent variables. You can set server_message_min to, say, > DEBUG or DEBUG4. > > Finally, I still object strongly to the notion that any of these message > levels should be hard wired as "never send to client" or "never send to > log"; if you can't imagine scenarios where people will want to override > that behavior, then your imagination is lacking. I want that choice to > be completely configurable. What I'd suggest is an exclusion mask. > Suppose we order the levels as > > PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR, > PGFATAL, PGCRASH. > > Then I think from the client's point of view the only knob needed is > client_message_min (settable between PGDEBUG4 and PGERROR, with default > PGINFO). On the server side we could have server_message_min (settable > between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a > list server_message_exclude of levels not to log, which could have > default value "PGINFO,PGNOTICE" (internally this would become a bitmask > so it would be cheap to test). > > It'd be possible to unify server_message_min and server_message_exclude > into a single parameter that's just a message level bitmask, but I think > the two separate parameters would be easier to deal with; otherwise you > end up with an awfully verbose parameter value. > > regards, tom lane > -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Actually, it is even simpler. Let's do this:
Client levels:
DEBUG, LOG, INFO, NOTICE, ERROR
Server levels:
DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH
Where client LOG shows LOG, INFO and higher, and server INFO shows INFO,
LOG and higher.
Not sure if INFOLOG was clearer or not.
---------------------------------------------------------------------------
Bruce Momjian wrote:
>
> OK, let me throw out a modified proposal that attempts to deal with
> these issues, and avoids creating a complicated error reporting system
> that few will understand or be able to control.
>
> How about if we have a new *_min_message setting, INFOLOG, which grabs
> client and server log info. This will allow a unified system where the
> client_min_message can be set to:
>
> DEBUG, INFOLOG, INFO, NOTICE, ERROR
>
> default, INFO, and the server_min_message can be:
>
> DEBUG, INFOLOG, LOG, NOTICE, ERROR, FATAL, CRASH
>
> default, LOG.
>
> Basically we do this:
>
> elog(INFO, ...)
>
> and this prints to the client if its value is INFO or less, and
> prints to the server if its level is INFOLOG or less. In this case:
>
> elog(LOG, ...)
>
> it prints to the client if its value is INFOLOG or less, and prints to
> the server if its level is LOG or less.
>
> This allows elog() to go to both places, if desired, and allows control
> in a fine-grained manner.
>
> This handles Tom's issue that DEBUG should be visible to the client if
> it wishes.
>
> Peter's issue of sending separate messages to client and server can be
> handled with two elog messages, one LOG, another INFO.
>
> This is all backward compatible because it does not remove any codes,
> only adds elog levels INFO and LOG, which are lower than NOTICE.
>
> Also, I like Tom's idea of DEBUG4 in the code, rather than testing the
> debug level. Instead, do the level testing in elog() function, which
> makes sense. This does allow DEBUGx as a possible debug level.
>
> ---------------------------------------------------------------------------
>
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > First, I think ERROR/DEBUG/NOTICE/FATAL, etc are too generic and cause
> > > compile problems/warnings, especially with Perl. I suggest renaming all
> > > elog levels to PG*, so it would be PGERROR and PGINFO. We could also do
> > > E_* or E*. I am interested in other opinions.
> >
> > We must put a prefix on these things. I have no strong opinion about
> > which prefix to use, but the change is long overdue.
> >
> > > Second, I propose adding two GUC variables that control how much elog()
> > > info is sent to the server and client logs. I suggest
> > > 'server_message_min' with possible values DEBUG, LOG, NOTICE, ERROR,
> > > FATAL, CRASH; and 'client_message_min' with possible values INFO,
> > > NOTICE, ERROR, FATAL, CRASH.
> >
> > client_message_min cannot usefully be set higher than ERROR. It will
> > certainly break libpq, for example, if 'E' messages don't come back when
> > a command fails. I also object to the idea that client_message_min
> > should not be settable to a level that allows DEBUG messages to be seen.
> > That would frequently save having to scrounge in the postmaster log,
> > which is not that easy if you're a client on a different machine.
> >
> > > We currently have 'debug_level' which controls how much debug
> > > information is sent to the server logs. I believe this would have to
> > > remain because it controls how _much_ DEBUG output is printed. We could
> > > go with some kind of hybrid where "DEBUG 5" sets DEBUG as the minimum
> > > reporting mode with level 5.
> >
> > I think you missed the point of my previous suggestion: let's invent
> > multiple DEBUG tags, for example DEBUG, DEBUG2, DEBUG3, DEBUG4,
> > and replace code like
> >
> > if (DebugLevel > 4)
> > elog(DEBUG, ...);
> >
> > with
> >
> > elog(PGDEBUG4, ...);
> >
> > This way, the log verbosity is controllable by a single mechanism rather
> > than two independent variables. You can set server_message_min to, say,
> > DEBUG or DEBUG4.
> >
> > Finally, I still object strongly to the notion that any of these message
> > levels should be hard wired as "never send to client" or "never send to
> > log"; if you can't imagine scenarios where people will want to override
> > that behavior, then your imagination is lacking. I want that choice to
> > be completely configurable. What I'd suggest is an exclusion mask.
> > Suppose we order the levels as
> >
> > PGDEBUG4, PGDEBUG3, PGDEBUG2, PGDEBUG, PGLOG, PGINFO, PGNOTICE, PGERROR,
> > PGFATAL, PGCRASH.
> >
> > Then I think from the client's point of view the only knob needed is
> > client_message_min (settable between PGDEBUG4 and PGERROR, with default
> > PGINFO). On the server side we could have server_message_min (settable
> > between PGDEBUG4 and PGCRASH, with default probably PGLOG) plus a
> > list server_message_exclude of levels not to log, which could have
> > default value "PGINFO,PGNOTICE" (internally this would become a bitmask
> > so it would be cheap to test).
> >
> > It'd be possible to unify server_message_min and server_message_exclude
> > into a single parameter that's just a message level bitmask, but I think
> > the two separate parameters would be easier to deal with; otherwise you
> > end up with an awfully verbose parameter value.
> >
> > regards, tom lane
> >
>
> --
> Bruce Momjian | http://candle.pha.pa.us
> pgman@candle.pha.pa.us | (610) 853-3000
> + If your life is a hard drive, | 830 Blythe Avenue
> + Christ can be your backup. | Drexel Hill, Pennsylvania 19026
>
> ---------------------------(end of broadcast)---------------------------
> TIP 6: Have you searched our list archives?
>
> http://archives.postgresql.org
>
-- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610)
853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill,
Pennsylvania19026
> OK, let me throw out a modified proposal that attempts to deal with > these issues, and avoids creating a complicated error reporting system > that few will understand or be able to control. Question: does any of this help with the problem (I assume it's a problem) that a syntax error in a statement causes the transaction to be rolled back? Very annoying in psql... Chris
Christopher Kings-Lynne wrote: > > OK, let me throw out a modified proposal that attempts to deal with > > these issues, and avoids creating a complicated error reporting system > > that few will understand or be able to control. > > Question: does any of this help with the problem (I assume it's a problem) > that a syntax error in a statement causes the transaction to be rolled back? > > Very annoying in psql... No, that is savepoints. I have a proposal on that in TODO.detail but not sure if it is acceptable or when I can implement it. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Actually, it is even simpler. Let's do this:
> Client levels:
> DEBUG, LOG, INFO, NOTICE, ERROR
> Server levels:
> DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH
Hmm, so the two cases have different ideas of the ordering of the
levels? Could be confusing, but it does keep the configuration
entries simple-looking.
What's your reaction to Peter's comments that the whole notion of
a linear set of elog levels should be abandoned?
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Actually, it is even simpler. Let's do this: > > Client levels: > > DEBUG, LOG, INFO, NOTICE, ERROR > > Server levels: > > DEBUG, INFO, LOG, NOTICE, ERROR, FATAL, CRASH > > Hmm, so the two cases have different ideas of the ordering of the > levels? Could be confusing, but it does keep the configuration > entries simple-looking. > > What's your reaction to Peter's comments that the whole notion of > a linear set of elog levels should be abandoned? I don't want to get into a second-system effect where we develop a system that is hard to manage. We do need error codes, and I think this system will fit into that when we decide to do it. However, we would still need a system of reporting control if we went with codes. I don't see a way around that. I have seen the linear error systems where everything is numbers, and things that are 9X are serious and -1X are not, but it seems quite confusing. Eventually we can base codes on these levels we have defined and go from there. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Tom Lane writes: > Peter Eisentraut <peter_e@gmx.net> writes: > > I think there should be separately named functions. > > You mean elog_debug(), elog_error(), ...? Seems pretty yucky. I would rather avoid the name "elog" altogether, for semantic and compatibility reasons. We could invent much nicer names, e.g., LogDebug(int, const char*, ...) LogInfo(const char*, ...) AbortTransaction(const char*, ...) AbortSession(const char*, ...) AbortAllSessions(const char*, ...) > > and we don't have to tag all strings as translatable. > > Huh? How does having multiple functions instead of one help there? The string extractor can only go by function name, not arguments. > I'm not sure that "type of error" really conveys much, or that we can > always tell what the cause of an error report is. What I mean with "type of error" is that there's a significant difference between user errors and server-side errors: 1. User errors should not necessarily go into the server log, unless command logging is enabled. 2. User errors will eventually carry additional information such as error codes. Server errors will just get one default error code. 3. Users should not necessarily be allowed to see the details of server errors at the client side, only some generic message. So if we made up two separate functions each for errors and notices, we could raise the awareness about this, even if initially the functionality would not differ much. > Also agreed. Probably the new function has to be called something else > than elog in any case, so that we can have a compatibility layer to > accept old elog calls. (That'd avoid requiring a "big bang" patch, too, > which'd surely ease making the changes.) Exactly. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: > Tom Lane writes: > > > Peter Eisentraut <peter_e@gmx.net> writes: > > > I think there should be separately named functions. > > > > You mean elog_debug(), elog_error(), ...? Seems pretty yucky. > > I would rather avoid the name "elog" altogether, for semantic and > compatibility reasons. We could invent much nicer names, e.g., > > LogDebug(int, const char*, ...) > LogInfo(const char*, ...) > AbortTransaction(const char*, ...) > AbortSession(const char*, ...) > AbortAllSessions(const char*, ...) Wow, seems awfully complex to me. Doesn't our current system seem clearer: elog(DEBUG, ...)elog(NOTICE, ...)elog(ERROR, ...) Our elog messages are long enough already. > > > and we don't have to tag all strings as translatable. > > > > Huh? How does having multiple functions instead of one help there? > > The string extractor can only go by function name, not arguments. Can't we hack it to pull out only certain elogs()? Also, don't we have to translate everything? I guess not. > > I'm not sure that "type of error" really conveys much, or that we can > > always tell what the cause of an error report is. > > What I mean with "type of error" is that there's a significant difference > between user errors and server-side errors: > > 1. User errors should not necessarily go into the server log, unless > command logging is enabled. > > 2. User errors will eventually carry additional information such as error > codes. Server errors will just get one default error code. > > 3. Users should not necessarily be allowed to see the details of server > errors at the client side, only some generic message. > > So if we made up two separate functions each for errors and notices, we > could raise the awareness about this, even if initially the functionality > would not differ much. Seems my solution is smaller and backward compatible. I don't see the value in tons of options. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Peter Eisentraut <peter_e@gmx.net> writes:
>> You mean elog_debug(), elog_error(), ...? Seems pretty yucky.
> I would rather avoid the name "elog" altogether, for semantic and
> compatibility reasons.
Okay; it was never a particularly nice name anyway. But ---
> We could invent much nicer names, e.g.,
> LogDebug(int, const char*, ...)
> LogInfo(const char*, ...)
> AbortTransaction(const char*, ...)
> AbortSession(const char*, ...)
> AbortAllSessions(const char*, ...)
Well, (a) AbortTransaction is taken; (b) I would like to see these names
kept as short as possible, to leave more room on the line for the error
message text. We could do something like this involving abbreviations,
I suppose, but the existing and proposed mnemonics (LOG, INFO, ...,
CRASH) seem perfectly usable to me.
>>> and we don't have to tag all strings as translatable.
>>
>> Huh? How does having multiple functions instead of one help there?
> The string extractor can only go by function name, not arguments.
Oh, now I get the point: you want to not pick up debug-level messages
for translation at all. That makes sense. Okay, how about we take
the names Bruce was proposing and make them function names:
PGLog(msg, ...)PGError(msg, ...)PGCrash(msg, ...)
regards, tom lane
> Oh, now I get the point: you want to not pick up debug-level messages > for translation at all. That makes sense. Okay, how about we take > the names Bruce was proposing and make them function names: > > PGLog(msg, ...) > PGError(msg, ...) > PGCrash(msg, ...) OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes sense. Should we consider hiding these in macros so they really still call elog(ERROR, ...) for backward compatiblity? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
> sense. Should we consider hiding these in macros so they really still
> call elog(ERROR, ...) for backward compatiblity?
I would love to make them macros, but I don't know a portable way to
create macros with variable numbers of arguments. Do you feel like
writing double parens?
PGERROR((msg, ...))
regards, tom lane
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes > > sense. Should we consider hiding these in macros so they really still > > call elog(ERROR, ...) for backward compatiblity? > > I would love to make them macros, but I don't know a portable way to > create macros with variable numbers of arguments. Do you feel like > writing double parens? > > PGERROR((msg, ...)) Then we have to wonder what PGError is getting us that elog(ERROR) isn't, except the ability to do internationalization based on the first parameter. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian writes: > Can't we hack it to pull out only certain elogs()? Also, don't we have > to translate everything? I guess not. I'm not sure. Someone other than me raised this point once. It's not so important. I supposed, eventually people will want to translate everything. Feel free to keep it as once function. > > What I mean with "type of error" is that there's a significant difference > > between user errors and server-side errors: > > > > 1. User errors should not necessarily go into the server log, unless > > command logging is enabled. > > > > 2. User errors will eventually carry additional information such as error > > codes. Server errors will just get one default error code. > > > > 3. Users should not necessarily be allowed to see the details of server > > errors at the client side, only some generic message. > > > > So if we made up two separate functions each for errors and notices, we > > could raise the awareness about this, even if initially the functionality > > would not differ much. > > Seems my solution is smaller and backward compatible. Your solution renumbers the error codes, so it's definitely not backward-compatible. > I don't see the value in tons of options. Well, I do. We don't need the separate user-side error functions initially, but eventually we will have to have them. So, basically, what this comes down to with respect to your patch: 1. Renumbering the error codes breaks backward compatibility *silently*. 2. CRASH doesn't seem like a good name to me. 3. I agree with adding a LOG or INFO level between DEBUG and NOTICE. 4. I don't like the alignment change. That seems very un-computer-like. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut wrote: > Bruce Momjian writes: > > > Can't we hack it to pull out only certain elogs()? Also, don't we have > > to translate everything? I guess not. > > I'm not sure. Someone other than me raised this point once. It's not so > important. I supposed, eventually people will want to translate > everything. Feel free to keep it as once function. OK. Glad it isn't a big issue. > > > What I mean with "type of error" is that there's a significant difference > > > between user errors and server-side errors: > > > > > > 1. User errors should not necessarily go into the server log, unless > > > command logging is enabled. > > > > > > 2. User errors will eventually carry additional information such as error > > > codes. Server errors will just get one default error code. > > > > > > 3. Users should not necessarily be allowed to see the details of server > > > errors at the client side, only some generic message. > > > > > > So if we made up two separate functions each for errors and notices, we > > > could raise the awareness about this, even if initially the functionality > > > would not differ much. > > > > Seems my solution is smaller and backward compatible. > > Your solution renumbers the error codes, so it's definitely not > backward-compatible. I don't need to renumber them. It is backward compatible at a source code level, not an object code level. Is object code backward compability for elog() an issue? If so, I don't need to renumber them. > > I don't see the value in tons of options. > > Well, I do. We don't need the separate user-side error functions > initially, but eventually we will have to have them. > > So, basically, what this comes down to with respect to your patch: > > 1. Renumbering the error codes breaks backward compatibility *silently*. Breaks object code only, which I think is minor, but I don't have to. > > 2. CRASH doesn't seem like a good name to me. Tom and I came up with that one. Feel free to suggest another. > 3. I agree with adding a LOG or INFO level between DEBUG and NOTICE. Good. > 4. I don't like the alignment change. That seems very un-computer-like. So you want two spaces after every colon, no matter what? Sure. I just makes the server logs jagged but it is a win on the user side. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Well, I do. We don't need the separate user-side error functions > initially, but eventually we will have to have them. > > So, basically, what this comes down to with respect to your patch: > > 1. Renumbering the error codes breaks backward compatibility *silently*. OK, so people want the old codes kept. I just have to warn people that there will be some funky test structure in elog.c to handle message *_min_messages tests. Is that OK? > 2. CRASH doesn't seem like a good name to me. OK, changed to FATALALL, which is pretty descriptive. > 3. I agree with adding a LOG or INFO level between DEBUG and NOTICE. OK, good, that was the crux of the patch. The rest was cleanups. > > 4. I don't like the alignment change. That seems very un-computer-like. OK, removed, not always two spaces. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Tom Lane wrote:
>
> Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
> > sense. Should we consider hiding these in macros so they really still
> > call elog(ERROR, ...) for backward compatiblity?
>
> I would love to make them macros, but I don't know a portable way to
> create macros with variable numbers of arguments. Do you feel like
> writing double parens?
>
> PGERROR((msg, ...))
I'm not sure about complete portability, but the trick that I use works
under both Linux/gcc and Windows/Visual C++. It is something like this:
#define elog mkError(__FILE__, __LINE__)
I then have the following prototypes:
----
typedef void (*throwError_ptr) (const char *message, ...);
throwError_ptr mkError(const char *file, int line);
void throwError(const char *message, ...);
-----
The implementation looks like:
throwError_ptr mkError(const char *file, int line) {
sprintf(global_message, "File: %s\nLine: %i\n", file, line);return (throwError)
}
void throwError(const char *message, ...) {
va_list arg_ptr;char buffer[MAX_MESSAGE];
va_start(arg_ptr, message);vsnprintf(buffer, sizeof(buffer), message, arg_ptr);va_end(arg_ptr);
fprintf(stderr, "Error: %s\n%s", global_message, message);
}
-----
So, I can write:
...
if (i < 0) {
elog("Illegal index specified: %i", i);
}
...
which becomes
mkError(__FILE__, __LINE__) ("Illegal index specified: %i", i);
Shouldn't that be portable?
Mike Mascari
mascarm@mascari.com
Peter Eisentraut <peter_e@gmx.net> writes:
> So, basically, what this comes down to with respect to your patch:
> 1. Renumbering the error codes breaks backward compatibility *silently*.
Perhaps, but it doesn't bother me. We have *never* promised binary
compatibility of server-side extensions across versions; usually,
you should be happy if a recompile is sufficient ;-). (Structs,
for example, are subject to field rearrangement all the time.)
In any case, we could maintain binary compatibility for the old-style
codes (DEBUG, ERROR, etc); this does not force us to use matching
codes for the new PG_ERROR etc. levels.
> 2. CRASH doesn't seem like a good name to me.
Why not? It's short, memorable, accurate, and what's wrong with
a little levity?
> 3. I agree with adding a LOG or INFO level between DEBUG and NOTICE.
Both, I think; they're not the same thing. LOG = routine server
operation notices (eg, "checkpoint starting now"). INFO =
allegedly-helpful messages issued to client (eg, the one about
truncating overlength identifiers). Normal configuration would
be to put one but not the other into the postmaster log.
regards, tom lane
...
> > 2. CRASH doesn't seem like a good name to me.
> Why not? It's short, memorable, accurate, and what's wrong with
> a little levity?
Because you'd have to be, uh, us to see humor in that message when your
system is not behaving :)
I'm with Peter on this one; let's find another more neutral name.
- Tom
Thomas Lockhart <lockhart@fourpalms.org> writes:
> I'm with Peter on this one; let's find another more neutral name.
Proposals then? What's been used or bandied about so far are
"REALLYFATAL" (yuck, even though I take the blame for it).
"STOP" (Vadim put this in, but I object to it as being too vague; it's not at all obvious that it's worse than
FATAL).
"FATALALL" (also yuck).
Surely we can find something that's short, memorable, and clearly
a notch more fatal than FATAL.
regards, tom lane
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> I would love to make them macros, but I don't know a portable way to
>> create macros with variable numbers of arguments. Do you feel like
>> writing double parens?
>>
>> PGERROR((msg, ...))
> Then we have to wonder what PGError is getting us that elog(ERROR)
> isn't, except the ability to do internationalization based on the first
> parameter.
The reason that I'd like a layer of macros in there is that it would
make it easier to handle acquisition of additional data about the
error. In particular, I've long wanted to pass __FILE__ and __LINE__
to the error handler so that the exact source code location of an
error report could be available. It's not reasonable to expect people
to write those in source code, but with a macro layer it'd be easy.
If we are really going to make a source code overhaul of the elog calls,
we ought not do it until we've thought about all the want-list items
that have been expressed in the past --- error codes, source locations,
cursor offsets, etc. I don't necessarily say we have to *do* all those
at once, but I think it's folly to do a massive overhaul without being
sure that we know what the future development path is going to be.
Otherwise we'll still be looking to do another massive edit after we've
gotten it right. Let's try to get it right beforehand, instead.
regards, tom lane
How about 'perdition'? Perhaps not common enough, but it seems to describe the situation pretty well. If you know the word it's way worse than fatal. Complete destruction, loss, ruin... -- Rod Taylor This message represents the official view of the voices in my head ----- Original Message ----- From: "Tom Lane" <tgl@sss.pgh.pa.us> To: "Thomas Lockhart" <lockhart@fourpalms.org> Cc: "Peter Eisentraut" <peter_e@gmx.net>; "Bruce Momjian" <pgman@candle.pha.pa.us>; "PostgreSQL-development" <pgsql-hackers@postgresql.org> Sent: Saturday, February 23, 2002 12:04 PM Subject: Re: [HACKERS] elog() proposal > Thomas Lockhart <lockhart@fourpalms.org> writes: > > I'm with Peter on this one; let's find another more neutral name. > > Proposals then? What's been used or bandied about so far are > "REALLYFATAL" (yuck, even though I take the blame for it). > "STOP" (Vadim put this in, but I object to it as being too vague; > it's not at all obvious that it's worse than FATAL). > "FATALALL" (also yuck). > > Surely we can find something that's short, memorable, and clearly > a notch more fatal than FATAL. > > regards, tom lane > > ---------------------------(end of broadcast)--------------------------- > TIP 1: subscribe and unsubscribe commands go to majordomo@postgresql.org >
Tom Lane wrote: > Bruce Momjian <pgman@candle.pha.pa.us> writes: > > Tom Lane wrote: > >> I would love to make them macros, but I don't know a portable way to > >> create macros with variable numbers of arguments. Do you feel like > >> writing double parens? > >> > >> PGERROR((msg, ...)) > > > Then we have to wonder what PGError is getting us that elog(ERROR) > > isn't, except the ability to do internationalization based on the first > > parameter. > > The reason that I'd like a layer of macros in there is that it would > make it easier to handle acquisition of additional data about the > error. In particular, I've long wanted to pass __FILE__ and __LINE__ > to the error handler so that the exact source code location of an > error report could be available. It's not reasonable to expect people > to write those in source code, but with a macro layer it'd be easy. > > If we are really going to make a source code overhaul of the elog calls, > we ought not do it until we've thought about all the want-list items > that have been expressed in the past --- error codes, source locations, > cursor offsets, etc. I don't necessarily say we have to *do* all those > at once, but I think it's folly to do a massive overhaul without being > sure that we know what the future development path is going to be. > Otherwise we'll still be looking to do another massive edit after we've > gotten it right. Let's try to get it right beforehand, instead. I guess that is the question. My changes are purely to control output location simply from GUC and to clean up some loose ends. It is backward compatible, at least at the source code level, which we already discussed. The question is whether we want a more major overhaul to return function/line number information. In most cases, this would be turned off, but in other cases we may want to enable it. I am not sure it is worth the effort, and frankly, I don't even want to touch something like that until just before beta when there are no outstanding patches and I have the tree to myself, like when I run pgindent. It is something to consider. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > So, basically, what this comes down to with respect to your patch: > > > 1. Renumbering the error codes breaks backward compatibility *silently*. > > Perhaps, but it doesn't bother me. We have *never* promised binary > compatibility of server-side extensions across versions; usually, > you should be happy if a recompile is sufficient ;-). (Structs, > for example, are subject to field rearrangement all the time.) I didn't think we had binary backward compatibility for server functions. The switch statement to test *_min_messages levels is going to look pretty bad, compared to a simple greater-than test for level values. One intestesting trick would be to start numbering the elog messages levels at 10, and throw an error if any messages comes in with a value less than that. That would properly warn people of old object files and may be the best bet. > In any case, we could maintain binary compatibility for the old-style > codes (DEBUG, ERROR, etc); this does not force us to use matching > codes for the new PG_ERROR etc. levels. Yes, I think keeping the old symbols in for one release is a good idea. Again, this would be done only just before beta, during pgindent run, where there are no outstanding patches. > > 2. CRASH doesn't seem like a good name to me. > > Why not? It's short, memorable, accurate, and what's wrong with > a little levity? > > > 3. I agree with adding a LOG or INFO level between DEBUG and NOTICE. > > Both, I think; they're not the same thing. LOG = routine server > operation notices (eg, "checkpoint starting now"). INFO = > allegedly-helpful messages issued to client (eg, the one about > truncating overlength identifiers). Normal configuration would > be to put one but not the other into the postmaster log. Sorry, truncating overly long identifiers is a NOTICE, not INFO. I reserved info for only places where the information did not indicate any kind of unusual situation, like SERIAL sequence creation. You can go through the existing NOTICE's after I apply and salt to taste. -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Karl DeBisschop wrote: > On Sat, 2002-02-23 at 12:04, Tom Lane wrote: > > Thomas Lockhart <lockhart@fourpalms.org> writes: > > > I'm with Peter on this one; let's find another more neutral name. > > > > Proposals then? What's been used or bandied about so far are > > "REALLYFATAL" (yuck, even though I take the blame for it). > > "STOP" (Vadim put this in, but I object to it as being too vague; > > it's not at all obvious that it's worse than FATAL). > > "FATALALL" (also yuck). > > > > Surely we can find something that's short, memorable, and clearly > > a notch more fatal than FATAL. > > PANIC ? MASTERFATAL? MASTERFAIL? -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
> Karl DeBisschop wrote:
>> PANIC ?
I like PANIC. If it's good enough for Unix kernels it's good enough
for us.
regards, tom lane
Tom Lane writes: > Thomas Lockhart <lockhart@fourpalms.org> writes: > > I'm with Peter on this one; let's find another more neutral name. > > Proposals then? What's been used or bandied about so far are > "REALLYFATAL" (yuck, even though I take the blame for it). > "STOP" (Vadim put this in, but I object to it as being too vague; > it's not at all obvious that it's worse than FATAL). > "FATALALL" (also yuck). I think FATALALL is good, because it tells you exactly what is going on, namely the same as FATAL but for all sessions. -- Peter Eisentraut peter_e@gmx.net
Karl DeBisschop writes: > PANIC ? PANIC tells you *absolutely nothing* about what is going to happen. Let's stop being cute and use something descriptive. It's not like we'll need to use it every four lines. -- Peter Eisentraut peter_e@gmx.net
Peter Eisentraut <peter_e@gmx.net> writes:
> I think FATALALL is good, because it tells you exactly what is going on,
> namely the same as FATAL but for all sessions.
But it isn't the same. If all backends FATAL'ed at once, that wouldn't
provoke the postmaster to wipe shared memory and run a WAL recovery
cycle.
What do you think of Karl's suggestion of PANIC?
regards, tom lane
Tom Lane wrote: > Peter Eisentraut <peter_e@gmx.net> writes: > > I think FATALALL is good, because it tells you exactly what is going on, > > namely the same as FATAL but for all sessions. > > But it isn't the same. If all backends FATAL'ed at once, that wouldn't > provoke the postmaster to wipe shared memory and run a WAL recovery > cycle. I called it FATALALL because the effect is to have all backends FATALly terminate. > What do you think of Karl's suggestion of PANIC? That is good too. The FATAL becomes like a process segfault, and PANIC is like a kernel panic. I don't have a preference. I will let you folks duke it out. :-) (I guess I lean toward PANIC. Sounds cooler. Not sure that is a good reason. :-) ) -- Bruce Momjian | http://candle.pha.pa.us pgman@candle.pha.pa.us | (610) 853-3000+ If your life is a hard drive, | 830 Blythe Avenue + Christ can be your backup. | Drexel Hill, Pennsylvania19026
Bruce Momjian <pgman@candle.pha.pa.us> writes:
> Tom Lane wrote:
>> But it isn't the same. If all backends FATAL'ed at once, that wouldn't
>> provoke the postmaster to wipe shared memory and run a WAL recovery
>> cycle.
> I called it FATALALL because the effect is to have all backends
> FATALly terminate.
But my point was that that's a very inadequate description of what it
does. Peter's insisting on precision in the names, so let's not pick
names that appear to mean something exact and are misleading about it.
>> What do you think of Karl's suggestion of PANIC?
> That is good too. The FATAL becomes like a process segfault, and PANIC
> is like a kernel panic.
Yeah, the more I think about it the more I like PANIC. There's really
a fairly good analogy to kernel panics: it's a forced system-wide
restart with ensuing recovery activity (WAL replay ~= fsck).
BTW, a more radical proposal would be to get rid of FATAL in its current
form altogether. FATAL essentially says "okay, this backend is so
screwed up that it can't possibly continue, but I'm certain we didn't
mess up shared memory". Now, how certain can you *really* be of that,
if you also believe that you can't recover the current backend?
Most of the existing uses of FATAL seem to be out-of-memory errors
(which these days ought not be considered FATAL), or startup errors
(which would more reasonably be handled by saying that ERROR before
we reach the main loop causes backend exit). There might be a small
number of places where it's really legitimate, but I think not very
many.
However, even if we did retire the current meaning of FATAL, I'd not
want to reassign it to STOP/PANIC/whatever; too much potential for
confusion if we do. So we need a new name for STOP anyway.
regards, tom lane
On Fri, Feb 22, 2002 at 07:57:54PM -0500, Bruce Momjian wrote:
> Tom Lane wrote:
> > Bruce Momjian <pgman@candle.pha.pa.us> writes:
> > > OK, so elog(ERROR, ...) and PGError(msg, ...) would be the same. Makes
> > > sense. Should we consider hiding these in macros so they really still
> > > call elog(ERROR, ...) for backward compatiblity?
> >
> > I would love to make them macros, but I don't know a portable way to
> > create macros with variable numbers of arguments. Do you feel like
> > writing double parens?
> >
> > PGERROR((msg, ...))
>
> Then we have to wonder what PGError is getting us that elog(ERROR)
> isn't, except the ability to do internationalization based on the first
> parameter.
I still try discover what is bad on elog() and I can't foundsomething relevant. Why do you need new name? Maybe use
pglog()if you want 'pg' prefix, or use pgevent() if you meanthat not all is "log".
Note about FATALALL: a lot of interpreted languages use "or die". What use: elog(DIE, ...).
I don't like upper case in frequent function names, an examplePGError(). IMHO upper case has effect for funtions
like"StartSomeImportantMasterProcess()"and not for function that isused on a lot of code lines.
Karel
-- Karel Zak <zakkr@zf.jcu.cz>http://home.zf.jcu.cz/~zakkr/C, PostgreSQL, PHP, WWW, http://docs.linux.cz,
http://mape.jcu.cz
> PANIC ?
OK with me...
- Thomas
On Sat, 2002-02-23 at 12:04, Tom Lane wrote: > Thomas Lockhart <lockhart@fourpalms.org> writes: > > I'm with Peter on this one; let's find another more neutral name. > > Proposals then? What's been used or bandied about so far are > "REALLYFATAL" (yuck, even though I take the blame for it). > "STOP" (Vadim put this in, but I object to it as being too vague; > it's not at all obvious that it's worse than FATAL). > "FATALALL" (also yuck). > > Surely we can find something that's short, memorable, and clearly > a notch more fatal than FATAL. PANIC ? -- Karl DeBisschop <kdebisschop@alert.infoplease.com> The Learning Network / Reference www.learningnetwork.com / www.infoplease.com