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: r1532085 - /jmeter/trunk/src/core/org/apache/jmeter/control/TransactionController.java
Date Sat, 23 Nov 2013 17:17:09 GMT
On 23 November 2013 16:22, Philippe Mouawad <philippe.mouawad@gmail.com> wrote:
> On Sat, Nov 23, 2013 at 4:41 PM, sebb <sebbaz@gmail.com> wrote:
>
>> On 23 November 2013 14:03, Philippe Mouawad <philippe.mouawad@gmail.com>
>> wrote:
>> > Hello,
>> > What about introducing a property called:
>> >
>> >    - transaction_controller.include_processing_and_timers
>> >
>> >
>> > It would allow users who have existing plans to stay with existing
>> > behaviour by setting:
>> >
>> >    - transaction_controller.include_processing_and_timers=true
>> >
>> >
>> > And default value would be false.
>>
>> That would change the behaviour for existing tests.
>>
> No if user sets  transaction_controller.include_processing_and_timers=true,
> everything remains as currently.
> We add a Incompatible change in changes.xml and everything is fine.

Yes, I understand that, but it still requires existing users to update
their installation.
Whereas if the default is true, only users who want the new behaviour
need to change their settings.

>
>> I'm not keen on changing the behaviour unless it is clearly incorrect.
>> I don't think that is the case here.
>>
> It is clearly incorrect because it reports in overall response time of
> Transaction Controller the time taken by post processors (I agree if it
> reported only timers it would be acceptable) .
>

It's still the overall time for the TC children.

If PostProcessors are taking a significant time to run, then that is
something to be looked at separately.
And there may be no PostProcessors.

>
>
>>
>> > By the way this impact should leed us to change something we've been
>> doing
>> > up till now which is not to write the value of a property if it is equal
>> to
>> > default.
>> > If we had not done this up until now , we could have changed this default
>> > with no impact .
>>
>> I don't think that is true.
>>
>> AFAICT it's possible for the JMX to have one default and the property
>> another.
>> However it would probably require the code to check whether the test
>> element is brand new or has been created from the JMX.
>>
>
> My proposition is to always write the value wether equals or not to
> default.

Which makes the JMX files much larger.
Also when the JMX file is saved it will change; if users store their
JMX files in an SCM system, it will generate unnecessary changes.

>>
>> It would also have to be carefully documented as we have not done this so
>> far.
>>
>> > Regards
>> > Philippe
>> >
>> >
>> >
>> > On Tue, Oct 15, 2013 at 4:46 PM, sebb <sebbaz@gmail.com> wrote:
>> >
>> >> On 14 October 2013 22:05, Philippe Mouawad <philippe.mouawad@gmail.com>
>> >> wrote:
>> >> > The problem is that if you change it , then automatically it will
>> change
>> >> > behaviour of current scripts I think.
>> >>
>> >> Yes.
>> >>
>> >> > As when reloading existing script,
>> >> > getPropertyAsBoolean(INCLUDE_TIMERS,
>> DEFAULT_VALUE_FOR_INCLUDE_TIMERS);
>> >> > will return DEFAULT_VALUE_FOR_INCLUDE_TIMERS , which will be false
>> while
>> >> in
>> >> > fact while its previous meaning when user saved his script was true.
>> >>
>> >> The value of the default cannot be changed without affecting existing
>> >> test plans.
>> >>
>> >> > See https://issues.apache.org/bugzilla/show_bug.cgi?id=55498
>> >> >
>> >> > In my opinion this should be changed adding an Incompatible change
>> but we
>> >> > would need to accept that users who really need it have to modify
>> their
>> >> > script.
>> >> >
>> >> > +1 for this breaking change.
>> >> >
>> >> > Regards
>> >> > Philippe
>> >> >
>> >> >
>> >> >
>> >> >
>> >> > On Mon, Oct 14, 2013 at 10:56 PM, Milamber <milamber@apache.org>
>> wrote:
>> >> >
>> >> >>
>> >> >> Le 14/10/2013 21:47, pmouawad@apache.org a ecrit :
>> >> >>
>> >> >>  Author: pmouawad
>> >> >>> Date: Mon Oct 14 20:47:58 2013
>> >> >>> New Revision: 1532085
>> >> >>>
>> >> >>> URL: http://svn.apache.org/r1532085
>> >> >>> Log:
>> >> >>> Add constant
>> >> >>>
>> >> >>
>> >> >> Perhaps, It would be great to transform this default value to a
>> property
>> >> >> for allow user to change this.
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >>
>> >> >>> Modified:
>> >> >>>      jmeter/trunk/src/core/org/**apache/jmeter/control/**
>> >> >>> TransactionController.java
>> >> >>>
>> >> >>> Modified: jmeter/trunk/src/core/org/**apache/jmeter/control/**
>> >> >>> TransactionController.java
>> >> >>> URL: http://svn.apache.org/viewvc/**jmeter/trunk/src/core/org/**
>> >> >>> apache/jmeter/control/**TransactionController.java?**
>> >> >>> rev=1532085&r1=1532084&r2=**1532085&view=diff<
>> >>
>> http://svn.apache.org/viewvc/jmeter/trunk/src/core/org/apache/jmeter/control/TransactionController.java?rev=1532085&r1=1532084&r2=1532085&view=diff
>> >> >
>> >> >>> ==============================**==============================**
>> >> >>> ==================
>> >> >>> ---
>> >>
>> jmeter/trunk/src/core/org/**apache/jmeter/control/**TransactionController.java
>> >> >>> (original)
>> >> >>> +++
>> >>
>> jmeter/trunk/src/core/org/**apache/jmeter/control/**TransactionController.java
>> >> >>> Mon Oct 14 20:47:58 2013
>> >> >>> @@ -53,6 +53,8 @@ public class TransactionController exten
>> >> >>>             private static final Logger log = LoggingManager.**
>> >> >>> getLoggerForClass();
>> >> >>>   +    private static final boolean
>> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS
>> >> =
>> >> >>> true; // default true for compatibility
>> >> >>> +
>> >> >>>       /**
>> >> >>>        * Only used in parent Mode
>> >> >>>        */
>> >> >>> @@ -293,7 +295,7 @@ public class TransactionController exten
>> >> >>>        * @param includeTimers
>> >> >>>        */
>> >> >>>       public void setIncludeTimers(boolean includeTimers) {
>> >> >>> -        setProperty(INCLUDE_TIMERS, includeTimers, true);
//
>> default
>> >> >>> true for compatibility
>> >> >>> +        setProperty(INCLUDE_TIMERS, includeTimers,
>> >> >>> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS);
>> >> >>>       }
>> >> >>>         /**
>> >> >>> @@ -302,6 +304,6 @@ public class TransactionController exten
>> >> >>>        * @return boolean (defaults to true for backwards
>> compatibility)
>> >> >>>        */
>> >> >>>       public boolean isIncludeTimers() {
>> >> >>> -        return getPropertyAsBoolean(INCLUDE_**TIMERS, true);
>> >> >>> +        return getPropertyAsBoolean(INCLUDE_**TIMERS,
>> >> >>> DEFAULT_VALUE_FOR_INCLUDE_**TIMERS);
>> >> >>>       }
>> >> >>>   }
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>>
>> >> >>
>> >> >
>> >> >
>> >> > --
>> >> > Cordialement.
>> >> > Philippe Mouawad.
>> >>
>> >
>> >
>> >
>> > --
>> > Cordialement.
>> > Philippe Mouawad.
>>
>
>
>
> --
> Cordialement.
> Philippe Mouawad.

Mime
View raw message