You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@httpd.apache.org by "William A. Rowe Jr." <wr...@rowe-clan.net> on 2010/06/08 20:19:38 UTC
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
-1 veto. This is not the appropriate test for a threaded MPM.
We have ap_mpm_query for this very purpose, please don't continue
to propagate this broken meme!!!
On 6/8/2010 11:26 AM, rjung@apache.org wrote:
> Author: rjung
> Date: Tue Jun 8 16:26:39 2010
> New Revision: 952724
>
> URL: http://svn.apache.org/viewvc?rev=952724&view=rev
> Log:
> Add descriptive prefix to pid and tid in the error log.
> Only log the tid, if the MPM is threaded.
>
> Suggested by jorton.
>
> Modified:
> httpd/httpd/trunk/server/log.c
>
> Modified: httpd/httpd/trunk/server/log.c
> URL: http://svn.apache.org/viewvc/httpd/httpd/trunk/server/log.c?rev=952724&r1=952723&r2=952724&view=diff
> ==============================================================================
> --- httpd/httpd/trunk/server/log.c (original)
> +++ httpd/httpd/trunk/server/log.c Tue Jun 8 16:26:39 2010
> @@ -628,12 +628,17 @@ static void log_error_core(const char *f
> "[%s] ", priorities[level_and_mask].t_name);
>
> len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
> - "[%" APR_PID_T_FMT, getpid());
> + "[pid %" 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);
> + int result;
> +
> + if (ap_mpm_query(AP_MPMQ_IS_THREADED, &result) == 0
> + && result == AP_MPMQ_STATIC) {
> + apr_os_thread_t tid = apr_os_thread_current();
> + len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
> + ":tid %pT", &tid);
> + }
> }
> #endif
> errstr[len++] = ']';
>
>
>
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
Posted by Rainer Jung <ra...@kippdata.de>.
Sorry for the noise, wrong list and language ...
On 08.06.2010 22:20, Rainer Jung wrote:
> On 08.06.2010 22:08, William A. Rowe Jr. wrote:
>> On 6/8/2010 1:19 PM, William A. Rowe Jr. wrote:
>>> -1 veto. This is not the appropriate test for a threaded MPM.
>>>
>>> We have ap_mpm_query for this very purpose, please don't continue
>>> to propagate this broken meme!!!
>>
>> I'm sorry - I didn't read this patch sufficiently (zeroed straight
>> into the APR_HAVE_THREADS test). However, there seems to be no reason
>> to exclude logging the threads with static builds(!?!)
>>
>> I'm not sure there's any reason to test AP_MPMQ_NOT_SUPPORTED either;
>> this should be a true/false since this query was introduced day one.
>
> Actually the OS2 and Netware MPMs return AP_MPMQ_DYNAMIC for that query.
> Although I never worked with those MPMs, the code indicates they are
> actually threaded, but the number of threads per process can vary.
>
> So checking against AP_MPMQ_NOT_SUPPORTED is safer.
>
> Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
Posted by Rainer Jung <ra...@kippdata.de>.
On 08.06.2010 22:08, William A. Rowe Jr. wrote:
> On 6/8/2010 1:19 PM, William A. Rowe Jr. wrote:
>> -1 veto. This is not the appropriate test for a threaded MPM.
>>
>> We have ap_mpm_query for this very purpose, please don't continue
>> to propagate this broken meme!!!
>
> I'm sorry - I didn't read this patch sufficiently (zeroed straight
> into the APR_HAVE_THREADS test). However, there seems to be no reason
> to exclude logging the threads with static builds(!?!)
>
> I'm not sure there's any reason to test AP_MPMQ_NOT_SUPPORTED either;
> this should be a true/false since this query was introduced day one.
Actually the OS2 and Netware MPMs return AP_MPMQ_DYNAMIC for that query.
Although I never worked with those MPMs, the code indicates they are
actually threaded, but the number of threads per process can vary.
So checking against AP_MPMQ_NOT_SUPPORTED is safer.
Rainer
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
Posted by "William A. Rowe Jr." <wr...@apache.org>.
On 6/8/2010 1:19 PM, William A. Rowe Jr. wrote:
> -1 veto. This is not the appropriate test for a threaded MPM.
>
> We have ap_mpm_query for this very purpose, please don't continue
> to propagate this broken meme!!!
I'm sorry - I didn't read this patch sufficiently (zeroed straight
into the APR_HAVE_THREADS test). However, there seems to be no reason
to exclude logging the threads with static builds(!?!)
I'm not sure there's any reason to test AP_MPMQ_NOT_SUPPORTED either;
this should be a true/false since this query was introduced day one.
Reviewing it now.
In any case, the presence or absence of a tid in the log, or of one
platform or the other shouldn't be blocking an alpha candidate :)
If something is broke, we fix in the next alpha.
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
Posted by "William A. Rowe Jr." <wr...@apache.org>.
On 6/8/2010 1:19 PM, William A. Rowe Jr. wrote:
> -1 veto. This is not the appropriate test for a threaded MPM.
>
> We have ap_mpm_query for this very purpose, please don't continue
> to propagate this broken meme!!!
I'm sorry - I didn't read this patch sufficiently (zeroed straight
into the APR_HAVE_THREADS test). However, there seems to be no reason
to exclude logging the threads with static builds(!?!)
I'm not sure there's any reason to test AP_MPMQ_NOT_SUPPORTED either;
this should be a true/false since this query was introduced day one.
Reviewing it now.
In any case, the presence or absence of a tid in the log, or of one
platform or the other shouldn't be blocking an alpha candidate :)
If something is broke, we fix in the next alpha.
Re: svn commit: r952724 - /httpd/httpd/trunk/server/log.c
Posted by Stefan Fritsch <sf...@sfritsch.de>.
On Tue, 8 Jun 2010, William A. Rowe Jr. wrote:
> -1 veto. This is not the appropriate test for a threaded MPM.
>
> We have ap_mpm_query for this very purpose, please don't continue
> to propagate this broken meme!!!
Can you please be more verbose about what exactly you are objecting to? I
think the check should not be
result == AP_MPMQ_STATIC
but
result != AP_MPMQ_NOT_SUPPORTED
Is that what you meant?
> On 6/8/2010 11:26 AM, rjung@apache.org wrote:
>> Author: rjung
>> Date: Tue Jun 8 16:26:39 2010
>> New Revision: 952724
>>
>> URL: http://svn.apache.org/viewvc?rev=952724&view=rev
>> #if APR_HAS_THREADS
>> {
>> - apr_os_thread_t tid = apr_os_thread_current();
>> - len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
>> - ":%pT", &tid);
>> + int result;
>> +
>> + if (ap_mpm_query(AP_MPMQ_IS_THREADED, &result) == 0
>> + && result == AP_MPMQ_STATIC) {
>> + apr_os_thread_t tid = apr_os_thread_current();
>> + len += apr_snprintf(errstr + len, MAX_STRING_LEN - len,
>> + ":tid %pT", &tid);
>> + }
>> }