Bruce Momjian wrote:
Andrew Dunstan wrote:
Bruce Momjian wrote:
Andrew Dunstan wrote:
I thought we had thrashed this out back in August. Certainly the only thing I recall seeing after I submitted the patch was some stylistic criticism from Neil, which I addressed in a revised patch.
Anyway, it is in principle doable. That's partly why I adopted a printf
style format string. There are some wrinkles, though:
. interaction with syslog pid/timestamp logging
Yes. If you use syslog, just don't ask for pid/timestamp and let syslog do it. Of course, right now we are able to send non-pid/timestamp to syslog _and_ send pid/timestamp to a log file, but that seems like a rare operation that doesn't justify keeping the various log parameters separate.
I'm OK with that as long as it is the consensus view. It does seem a little odd to remove functionality (however rare) for the sake of configuration neatness, though.
Yes, agreed. Let's explore it. The rare functionality we would be removing is for: o User uses syslog o User wants to log postmaster output to a file too o User wants timestamp info in the postmaster output file
And the downside is that they get duplicate timestamps in syslog.
Now, if we don't merge timestamp into your new per-line log string, we end up with a rather illogical and inflexible output format, only to allow the rare case listed above.
Clearly this isn't a 100% clear decision, but it seems to lean in the direction of removing the functionality with the goal of providing a more logical logging API to the users.
This requires some thought. ISTM it wouldn't buy you much unless you made it persistent across server restarts, and possibly not even then. I don't see it as a reason to hold up these features, though. If someone wants to tackle this it could be plugged in to the loginfo feature very easily as an extra escape sequence.Also, I would like to see some kind of session identifier that is more unique than pid, which wraps around. Ideally we could have 10{pid}, then then the pid wraps around, 20{pid), or something like that.
Yes, that was my idea --- let's get this in and we can then add a session id, and your point about restarts is a good one.
If users want a non-empty info string they will have to teach the tools to handle it anyway. The point was, however, that rolling up PID and timestamp into the printf-style format will require some significant work, because we wouldn't want to lose that info if the user/db weren't known, whereas the patch currently suppresses all log-info output if these are not present (i.e. when MyProcPort == NULL).. making sure the info is available when you need to log it - I had to
rearrange a few thing to avoid getting SEGVs, IIRC.
Of course some messages, like postmaster status messages, don't have some of these fields, like username or host. Is that going to cause problems for tools that read our log files?
Oh, good point. That would suggest that maybe we are better off leaving pid and timestamp as separate options and _not_ merge them into your new string. I am starting to think having your string print only session-specific information is the best way.
I wonder if we should rename your GUC log_session_line or something like that.
No, I meant the logging of the end of a session, including its duration, which was also in the patch.Also, the session duration logging part of the patch is orthogonal to theYou mean query duration? Yes, it is orthoginal.
issue.
Oh, I missed that one. It seems like a nice addition. I see you added it when they ask for log_connections. Good idea.
I think you are looking at the original patch, not the revised patch, which is here: http://candle.pha.pa.us/mhonarc/patches2/msg00091.html and provides a separate GUC var called "log_session_end" - Neil wanted these not to be combined, IIRC.
I am agnostic as to the names of the variables.
cheers
andrew
---------------------------(end of broadcast)---------------------------
TIP 9: the planner will ignore your desire to choose an index scan if your
joining column's datatypes do not match
