jmeter-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Philippe Mouawad <philippe.moua...@gmail.com>
Subject Re: svn commit: r1532085 - /jmeter/trunk/src/core/org/apache/jmeter/control/TransactionController.java
Date Sat, 23 Nov 2013 22:59:19 GMT
Hello sebb,
My answers below.


On Sat, Nov 23, 2013 at 6:17 PM, sebb <sebbaz@gmail.com> wrote:

> 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.
>

See https://issues.apache.org/bugzilla/show_bug.cgi?id=55816 attached Test
Plan.
If the default is true, do you find it OK that users need to update their
installation to have the best behaviour from JMeter when using TC ?

I don't. Default should be the best ones, and backward compatibility should
not be more important than having the right behaviour.
I find it already very good that we introduce a special property to keep
old behaviour.


>
> >
> >> 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.
>

Yes but do you agree that a user only wants cumulated response time
excluding any JMeter processing.


> 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.
>



-- 
Cordialement.
Philippe Mouawad.

Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message