You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@tomcat.apache.org by Konstantin Kolinko <kn...@gmail.com> on 2011/07/06 08:03:01 UTC

Re: svn commit: r1135088 - in /tomcat/trunk: java/org/apache/coyote/AbstractProtocol.java webapps/docs/changelog.xml

1. JavaDoc for the method has to be updated. It still says that the
name is quoted.

2. If I understand correctly, the name _is_ used directly, in
AbstractConnectionHandler.register():
... ",worker=" + getProtocol().getName() + ",name=" ...

and MBeanUtils.destroyMBean(Connector,Service)
  worker = ((**Protocol)*).getName()
  new ObjectName(domain + ":type=RequestProcessor,worker=" + worker + ",*")


So if quotes are to be removed, let's do before appending the "-exec-"
suffix to them.
The name is passed to Endpoint (AbstractEndpoint.setName()) and the
"-exec-" suffix is added in AbstractEndpoint.createExecutor().  There
are also other suffixes elsewhere ("-Poller-", "-CometPoller-"), ...


2011/6/13  <rj...@apache.org>:
> Author: rjung
> Date: Mon Jun 13 11:19:23 2011
> New Revision: 1135088
>
> URL: http://svn.apache.org/viewvc?rev=1135088&view=rev
> Log:
> Remove superfluous quotes from thread names for
> connection pools.
>
> Example broken thread name: "http-apr-8001"-exec-2
> (including leading and intermediate quotes).
>
> Since we never use the names as a full ObjectName,
> only as a part of an ObjectName, it is safe to
> remove the surrounding quotes from the Protocol name.
>
> Modified:
>    tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
>    tomcat/trunk/webapps/docs/changelog.xml
>
> Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1135088&r1=1135087&r2=1135088&view=diff
> ==============================================================================
> --- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
> +++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Mon Jun 13 11:19:23 2011
> @@ -229,7 +229,8 @@ public abstract class AbstractProtocol i
>             name.append('-');
>         }
>         name.append(endpoint.getPort());
> -        return ObjectName.quote(name.toString());
> +        String quotedName = ObjectName.quote(name.toString());
> +        return quotedName.substring(1, quotedName.length()-1);
>     }
>
>
>
> Modified: tomcat/trunk/webapps/docs/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1135088&r1=1135087&r2=1135088&view=diff
> ==============================================================================
> --- tomcat/trunk/webapps/docs/changelog.xml (original)
> +++ tomcat/trunk/webapps/docs/changelog.xml Mon Jun 13 11:19:23 2011
> @@ -53,6 +53,10 @@
>         Fix unit test for bindOnInit which was failing for APR on some
>         platforms. (rjung)
>       </fix>
> +      <fix>
> +        Remove superfluous quotes from thread names for connection pools.
> +        (rjung)
> +      </fix>
>     </changelog>
>   </subsection>
>   <subsection name="Cluster">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
> For additional commands, e-mail: dev-help@tomcat.apache.org
>
>

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1135088 - in /tomcat/trunk: java/org/apache/coyote/AbstractProtocol.java webapps/docs/changelog.xml

Posted by Rainer Jung <ra...@kippdata.de>.
On 06.07.2011 10:07, Konstantin Kolinko wrote:
> 2011/7/6 Rainer Jung <ra...@kippdata.de>:
>> Hi Konstantin,
>>
>> On 06.07.2011 08:03, Konstantin Kolinko wrote:
>>> 1. JavaDoc for the method has to be updated. It still says that the
>>> name is quoted.
>>
>> Will fix.
>>
>>> 2. If I understand correctly, the name _is_ used directly, in
>>> AbstractConnectionHandler.register():
>>> ... ",worker=" + getProtocol().getName() + ",name=" ...
>>
>> It is used in ObjectNames, but only as a substring, not as a *full*
>> Objectname. That's why I left the quoting in the sense of
>> ObjectName.quote() in there, which might replace some special
>> characters, and only stripped the surrounding quotes from the result.
>>
>> An exaple for the resultig ObjectName is:
>>
>> Catalina:type=RequestProcessor,worker=http-bio-8080,name=HttpRequest1
>>
>> instead of
>>
>> Catalina:type=RequestProcessor,worker="http-bio-8080",name=HttpRequest1
>>
>>> and MBeanUtils.destroyMBean(Connector,Service)
>>>   worker = ((**Protocol)*).getName()
>>>   new ObjectName(domain + ":type=RequestProcessor,worker=" + worker + ",*")
>>>
>>>
>>> So if quotes are to be removed, let's do before appending the "-exec-"
>>> suffix to them.
>>
>> That's what should already happen. From JMX:
>>
>> workerThreadName: http-bio-8080-exec-1
>>
>> Thread names from thread dump:
>>
>> "ajp-bio-8009-AsyncTimeout"
>> "ajp-bio-8009-Acceptor-0"
>> "http-bio-8080-AsyncTimeout"
>> "http-bio-8080-Acceptor-0"
>>
>> etc.
>>
>>> The name is passed to Endpoint (AbstractEndpoint.setName()) and the
>>> "-exec-" suffix is added in AbstractEndpoint.createExecutor().  There
>>> are also other suffixes elsewhere ("-Poller-", "-CometPoller-"), ...
>>
>> Maybe you misinterpreted the multiple use of the word "quote". We still
>> quote in the sense of escaping some chars in the name if it wouldn't be
>> allowed in an ObjectName. We no longer quote in the sense of surrounding
>> the name with quotes. So all names derived from getName() should be fine
>> now.
>>
> 
> Thank you for the explanation.
> 
> From ObjectName javadoc:
> http://download.oracle.com/javase/1.5.0/docs/api/javax/management/ObjectName.html
> 
> [[[
> Each value associated with a key is a string of characters that is
> either unquoted or quoted.
> 
> An unquoted value is a possibly empty string of characters which may
> not contain any of the characters comma, equals, colon, quote,
> asterisk, or question mark.
> 
> A quoted value consists of a quote ("), followed by a possibly empty
> string of characters, followed by another quote.(...)
> ]]]
> 
> So it looks like the quotes should be used when building a name from parts.
> 
> Regarding the name itself - it may contains address, and address may
> contain a colon (':') if it is an ip6 address. That will require
> quoting as a colon cannot be present in an unquoted name.

To complete this discussion in case someone reads the thread in the
future: yes you were right. The change broke use of IPV6 addresses in
the Connector.

I reverted r1135088 and only removed the surrounding quotes from the
endpoint name in r1144785. This will also remove them from the thread
names, were they were annoying because of there placement in the middle
of the names.

Thread names are up to now not part of MBean names, only MBean attribute
values, so this should be save.

Sorry for not having checked this more carefully.

Regards,

Rainer



---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1135088 - in /tomcat/trunk: java/org/apache/coyote/AbstractProtocol.java webapps/docs/changelog.xml

Posted by Konstantin Kolinko <kn...@gmail.com>.
2011/7/6 Rainer Jung <ra...@kippdata.de>:
> Hi Konstantin,
>
> On 06.07.2011 08:03, Konstantin Kolinko wrote:
>> 1. JavaDoc for the method has to be updated. It still says that the
>> name is quoted.
>
> Will fix.
>
>> 2. If I understand correctly, the name _is_ used directly, in
>> AbstractConnectionHandler.register():
>> ... ",worker=" + getProtocol().getName() + ",name=" ...
>
> It is used in ObjectNames, but only as a substring, not as a *full*
> Objectname. That's why I left the quoting in the sense of
> ObjectName.quote() in there, which might replace some special
> characters, and only stripped the surrounding quotes from the result.
>
> An exaple for the resultig ObjectName is:
>
> Catalina:type=RequestProcessor,worker=http-bio-8080,name=HttpRequest1
>
> instead of
>
> Catalina:type=RequestProcessor,worker="http-bio-8080",name=HttpRequest1
>
>> and MBeanUtils.destroyMBean(Connector,Service)
>>   worker = ((**Protocol)*).getName()
>>   new ObjectName(domain + ":type=RequestProcessor,worker=" + worker + ",*")
>>
>>
>> So if quotes are to be removed, let's do before appending the "-exec-"
>> suffix to them.
>
> That's what should already happen. From JMX:
>
> workerThreadName: http-bio-8080-exec-1
>
> Thread names from thread dump:
>
> "ajp-bio-8009-AsyncTimeout"
> "ajp-bio-8009-Acceptor-0"
> "http-bio-8080-AsyncTimeout"
> "http-bio-8080-Acceptor-0"
>
> etc.
>
>> The name is passed to Endpoint (AbstractEndpoint.setName()) and the
>> "-exec-" suffix is added in AbstractEndpoint.createExecutor().  There
>> are also other suffixes elsewhere ("-Poller-", "-CometPoller-"), ...
>
> Maybe you misinterpreted the multiple use of the word "quote". We still
> quote in the sense of escaping some chars in the name if it wouldn't be
> allowed in an ObjectName. We no longer quote in the sense of surrounding
> the name with quotes. So all names derived from getName() should be fine
> now.
>

Thank you for the explanation.

>From ObjectName javadoc:
http://download.oracle.com/javase/1.5.0/docs/api/javax/management/ObjectName.html

[[[
Each value associated with a key is a string of characters that is
either unquoted or quoted.

An unquoted value is a possibly empty string of characters which may
not contain any of the characters comma, equals, colon, quote,
asterisk, or question mark.

A quoted value consists of a quote ("), followed by a possibly empty
string of characters, followed by another quote.(...)
]]]

So it looks like the quotes should be used when building a name from parts.

Regarding the name itself - it may contains address, and address may
contain a colon (':') if it is an ip6 address. That will require
quoting as a colon cannot be present in an unquoted name.

Best regards,
Konstantin Kolinko

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org


Re: svn commit: r1135088 - in /tomcat/trunk: java/org/apache/coyote/AbstractProtocol.java webapps/docs/changelog.xml

Posted by Rainer Jung <ra...@kippdata.de>.
Hi Konstantin,

On 06.07.2011 08:03, Konstantin Kolinko wrote:
> 1. JavaDoc for the method has to be updated. It still says that the
> name is quoted.

Will fix.

> 2. If I understand correctly, the name _is_ used directly, in
> AbstractConnectionHandler.register():
> ... ",worker=" + getProtocol().getName() + ",name=" ...

It is used in ObjectNames, but only as a substring, not as a *full*
Objectname. That's why I left the quoting in the sense of
ObjectName.quote() in there, which might replace some special
characters, and only stripped the surrounding quotes from the result.

An exaple for the resultig ObjectName is:

Catalina:type=RequestProcessor,worker=http-bio-8080,name=HttpRequest1

instead of

Catalina:type=RequestProcessor,worker="http-bio-8080",name=HttpRequest1

> and MBeanUtils.destroyMBean(Connector,Service)
>   worker = ((**Protocol)*).getName()
>   new ObjectName(domain + ":type=RequestProcessor,worker=" + worker + ",*")
> 
> 
> So if quotes are to be removed, let's do before appending the "-exec-"
> suffix to them.

That's what should already happen. From JMX:

workerThreadName: http-bio-8080-exec-1

Thread names from thread dump:

"ajp-bio-8009-AsyncTimeout"
"ajp-bio-8009-Acceptor-0"
"http-bio-8080-AsyncTimeout"
"http-bio-8080-Acceptor-0"

etc.

> The name is passed to Endpoint (AbstractEndpoint.setName()) and the
> "-exec-" suffix is added in AbstractEndpoint.createExecutor().  There
> are also other suffixes elsewhere ("-Poller-", "-CometPoller-"), ...

Maybe you misinterpreted the multiple use of the word "quote". We still
quote in the sense of escaping some chars in the name if it wouldn't be
allowed in an ObjectName. We no longer quote in the sense of surrounding
the name with quotes. So all names derived from getName() should be fine
now.

Regards,

Rainer

> 2011/6/13  <rj...@apache.org>:
>> Author: rjung
>> Date: Mon Jun 13 11:19:23 2011
>> New Revision: 1135088
>>
>> URL: http://svn.apache.org/viewvc?rev=1135088&view=rev
>> Log:
>> Remove superfluous quotes from thread names for
>> connection pools.
>>
>> Example broken thread name: "http-apr-8001"-exec-2
>> (including leading and intermediate quotes).
>>
>> Since we never use the names as a full ObjectName,
>> only as a part of an ObjectName, it is safe to
>> remove the surrounding quotes from the Protocol name.
>>
>> Modified:
>>    tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
>>    tomcat/trunk/webapps/docs/changelog.xml
>>
>> Modified: tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java?rev=1135088&r1=1135087&r2=1135088&view=diff
>> ==============================================================================
>> --- tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java (original)
>> +++ tomcat/trunk/java/org/apache/coyote/AbstractProtocol.java Mon Jun 13 11:19:23 2011
>> @@ -229,7 +229,8 @@ public abstract class AbstractProtocol i
>>             name.append('-');
>>         }
>>         name.append(endpoint.getPort());
>> -        return ObjectName.quote(name.toString());
>> +        String quotedName = ObjectName.quote(name.toString());
>> +        return quotedName.substring(1, quotedName.length()-1);
>>     }
>>
>>
>>
>> Modified: tomcat/trunk/webapps/docs/changelog.xml
>> URL: http://svn.apache.org/viewvc/tomcat/trunk/webapps/docs/changelog.xml?rev=1135088&r1=1135087&r2=1135088&view=diff
>> ==============================================================================
>> --- tomcat/trunk/webapps/docs/changelog.xml (original)
>> +++ tomcat/trunk/webapps/docs/changelog.xml Mon Jun 13 11:19:23 2011
>> @@ -53,6 +53,10 @@
>>         Fix unit test for bindOnInit which was failing for APR on some
>>         platforms. (rjung)
>>       </fix>
>> +      <fix>
>> +        Remove superfluous quotes from thread names for connection pools.
>> +        (rjung)
>> +      </fix>
>>     </changelog>
>>   </subsection>
>>   <subsection name="Cluster">


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@tomcat.apache.org
For additional commands, e-mail: dev-help@tomcat.apache.org