On 1 November 2012 11:59, sebb <[email protected]> wrote:
> On 29 October 2012 08:16, Milamber <[email protected]> wrote:
>>
>>
>> Le 29/10/2012 00:54, sebb a ecrit :
>>>
>>> On 26 October 2012 18:04,<[email protected]> 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.
Another alternative: rather than use setCookieManager etc, the pooling
code could set the property directly.
e.g. add the method
private void setCookieManagerProperty(CookieManager value) {
setProperty(new TestElementProperty(COOKIE_MANAGER, value));
}
this can then be called by the original public method:
public void setCookieManager(CookieManager value) {
CookieManager mgr = getCookieManager();
if (mgr != null) {
log.warn("Existing CookieManager " + mgr.getName() + "
superseded by " + value.getName());
}
setCookieManagerProperty(value);
}
[Similarly for setCacheManager]
The pooling code is then changed to use setCookieManagerProperty
rather than setCookieManager.
This bypasses the checks without needing an extra boolean parameter.
>>
>>
>>>
>>> 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));
>>>>
>>>>
>>
