jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From sebb <seb...@gmail.com>
Subject Re: svn commit: r1402574 - /jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
Date Thu, 01 Nov 2012 11:59:59 GMT
On 29 October 2012 08:16, Milamber <milamber@apache.org> wrote:
>
>
> Le 29/10/2012 00:54, sebb a ecrit :
>>
>> On 26 October 2012 18:04,<milamber@apache.org>  wrote:
>>>
>>> Author: milamber
>>> Date: Fri Oct 26 17:04:00 2012
>>> New Revision: 1402574
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1402574&view=rev
>>> Log:
>>> When used the options Retrieve All Embedded Resources + Concurrent pool,
>>> these log.warn() generates a lot of lines in jmeter.log.
>>>
>>> Modified:
>>>
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
>>
>> -1, see below.
>>
>>> Modified:
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
>>> URL:
>>> http://svn.apache.org/viewvc/jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java?rev=1402574&r1=1402573&r2=1402574&view=diff
>>>
>>> ==============================================================================
>>> ---
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
>>> (original)
>>> +++
>>> jmeter/trunk/src/protocol/http/org/apache/jmeter/protocol/http/sampler/HTTPSamplerBase.java
>>> Fri Oct 26 17:04:00 2012
>>> @@ -753,7 +753,7 @@ public abstract class HTTPSamplerBase ex
>>>
>>>       public void setAuthManager(AuthManager value) {
>>>           AuthManager mgr = getAuthManager();
>>> -        if (mgr != null) {
>>> +        if (log.isDebugEnabled()&&  mgr != null) {
>>>
>>>               log.warn("Existing AuthManager " + mgr.getName() + "
>>> superseded by " + value.getName());
>>
>> I'm not very happy with this fix, as it suppresses the warnings when
>> Concurrent pool is not in use.
>> It also presumably does not display valid warnings if concurrent pool is
>> in use.
>>
>> I've not looked at the code yet, but there has to be a better way to do
>> this.
>> Ideally change the pool code so it does not generate the overrides.
>
>
> In attachment a patch to fix the first incorrect fix.
> * revert on setAuthManager(), it's not necessary
> * add 2 new methods with a boolean parameter if call from the concurrent
> pool.
>
> It's a good way? or another better way exists?

That is definitely better, but will it allow genuine duplicates to be
detected when using pooling?
I suspect it will, but have not tested this.

Another possibility might be to allow setCookieManager etc. to allow a
null argument to reset the value without logging a warning.

The pooling code could then use:

setCookieManager(null);
setCookieManager(clonedCookieManager);

Similarly for the other managers.

However this is slightly less efficient as it updates the property twice.

If we think there are other use cases for allowing the manager value
to be nullified then we would need to make the changes to the
setManager methods anyway.
At present, it looks like the code will generate an NPE when building
the log message.

>
>
>>
>> Also, the code looks odd - why check for debug yet report a warning?
>>
>>>           }
>>>           setProperty(new TestElementProperty(AUTH_MANAGER, value));
>>> @@ -783,7 +783,7 @@ public abstract class HTTPSamplerBase ex
>>>
>>>       public void setCookieManager(CookieManager value) {
>>>           CookieManager mgr = getCookieManager();
>>> -        if (mgr != null) {
>>> +        if (log.isDebugEnabled()&&  mgr != null) {
>>>
>>>               log.warn("Existing CookieManager " + mgr.getName() + "
>>> superseded by " + value.getName());
>>>           }
>>>           setProperty(new TestElementProperty(COOKIE_MANAGER, value));
>>> @@ -795,7 +795,7 @@ public abstract class HTTPSamplerBase ex
>>>
>>>       public void setCacheManager(CacheManager value) {
>>>           CacheManager mgr = getCacheManager();
>>> -        if (mgr != null) {
>>> +        if (log.isDebugEnabled()&&  mgr != null) {
>>>
>>>               log.warn("Existing CacheManager " + mgr.getName() + "
>>> superseded by " + value.getName());
>>>           }
>>>           setProperty(new TestElementProperty(CACHE_MANAGER, value));
>>>
>>>
>

Mime
View raw message