On 08.06.2010 12:11, "Plüm, Rüdiger, VF-Group" wrote:
-----Original Message-----
From: Rainer Jung
Sent: Dienstag, 8. Juni 2010 11:14
To: dev@httpd.apache.org
Subject: Re: svn commit: r952201 - /httpd/httpd/trunk/server/log.c
On 08.06.2010 10:21, Joe Orton wrote:
On Mon, Jun 07, 2010 at 12:23:26PM -0000, rj...@apache.org wrote:
Author: rjung
Date: Mon Jun 7 12:23:26 2010
New Revision: 952201
URL: http://svn.apache.org/viewvc?rev=952201&view=rev
Log:
Add process id and thread id (if APR has thread support)
to the error log.
...
@@ -620,6 +621,18 @@ static void log_error_core(const char *f
if ((level& APLOG_STARTUP) != APLOG_STARTUP) {
len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
"[%s] ",
priorities[level_and_mask].t_name);
+
+ len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
+ "[%" APR_PID_T_FMT, getpid());
+#if APR_HAS_THREADS
+ {
+ apr_os_thread_t tid = apr_os_thread_current();
+ len += apr_snprintf(errstr + len,
MAX_STRING_LEN - len,
+ ":%pT",&tid);
+ }
+#endif
+ errstr[len++] = ']';
+ errstr[len++] = ' ';
These numbers made me double-take when reading error_log since they
don't have a descriptive prefix. Also the tid is not much use in a
non-threaded server. Could we do something like this?
(Hopefully this
function won't start showing up in CPU profiles!)
I'm neutral on adding the descriptive prefixes (+0). It's
helpful to get
acquainted with the new format, on the long term it might become
unnecessary. I guess when adding new standard tokens to the
error log we
might end up doing a callback based configurable format like in
mod_log_config.
Removing the tid for non-threaded servers is fine. I thought
about that
too. Would it make sense to somehow cache the result of
ap_mpm_query(AP_MPMQ_IS_THREADED,&result) in one of the structs
available from the log function, because it won't change during the
runtime of the instance (at least not during the runtime of
the child;
don't know whether it would be feasible to switch the new dynamically
loaded MPMs during restart, but that's another topic).
How about a local static variable in the function?
I tried it. But:
you can actually switch between MPMs now by apachectl graceful or
restart when building them dynamically. The static in the children comes
via fork from the parent process. The naive way of initializing it to
UNSET and then setting it once to the result of the MPM query doesn't
work, because it will miss any changes due to restart.
If we keep the variable local in the log function, then it would have to
check whenever it logs whether a restart has happened. I assume querying
the MPM isn't more expensive.
Rainer
