On 1 November 2012 14:23, sebb <[email protected]> wrote:
> 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.
I've tested this and it works.
The unnecessary warnings are no longer generated, but real warnings are logged.
So I have committed the code.
>>>
>>>
>>>>
>>>> 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));
>>>>>
>>>>>
>>>
