Bug 51480

Summary: ConstantThroughputTimer in shared mode all threads start with an incorrect delay of 0
Product: JMeter Reporter: baerrach <baerrach>
Component: MainAssignee: JMeter issues mailing list <issues>
Status: NEW ---    
Severity: normal CC: baerrach, p.mouawad
Priority: P2 Keywords: PatchAvailable
Version: 2.4   
Target Milestone: ---   
Hardware: All   
OS: All   
Attachments: Fix with unit tests
Trunk Patch
ConstantThroughputTimer.java
PackageTest.java
Simple test script

Description baerrach 2011-07-06 01:05:46 UTC
In shared mode:
* calcMode.4=all active threads (shared)
* calcMode.5=all active threads in current thread group (shared)
always has a previousTime of 0 for all threads.

This causes the first run of each thread to not use the "shared" times.

I've provided a patch, with unit tests.

A summary of the changes are below

ConstantThroughputTimer
* Changed threadGroupsInfoMap from Map to ConcurrentHashMap (it was using a ConcurrentHashMap implementation but exposing it as a Map) and putIfAbsent() is needed
* increased serialVersionUID as internal structure has changed.
* Moved ThroughputInfo to its own class and gave it a rewrite
* add constants for calc modes.
* increased MILLISEC_PER_MIN to protected visibility
* replaced previousTime with an instance of ThroughputInfo (this unifies the Timer for all use cases)
* added documentation where it was missing
* delay() delegates to throughputInfo
* deleted calculateCurrentTarget() - the comment indicated it was only used in test code and it no longer is
* renamed calculateDelay to calculateDelayForMode
* moved implementationd details of calculateSharedDelay to ThroughputInfo
* reset no longer resets class variables this is done in testEnded
* reset calls setCalcMode() to ensure that sharing of ThroughputInfo instances are done correctly for the shared modes

ThroughputInfo
* uses a static class SystemClock (which delegates to System.currentTimeMillis) to allow for unit testing
* previousTime from ConstantThroughputTimer renamed to lastScheduledTime
* lastScheduledTime is guarded by "this" instead of a Mutex.
* ConstantThroughputTimer.calculateSharedDelay and ConstantThroughputTimer.delay did the same things but with a different implementation, this is unified into calculateDelay()

== Testing == 
* MockJMeterContext created that delegates all calls to an instance of JMeterContext, used to switch out the current context when "activated" as unit testing doesn't setup threads needed for ThreadLocal context variables.
* Created a simulation class to make creating scenarios from a file possible
* Created ConstantThroughputTimerTest with some more thorough testing scenarios
* Created ConstantThroughputTimerTestData which holds all the test data needed to hide the complexity of setting up unit tests
* Upgraded PackageTest to use SimulationClock instead of relying on Thread.sleep and timing to test
* SimulationClock to control SystemClock timings during test
* SimulationEvent defines what offset the event occurs at and the expected delay
* simulator_mode*.txt files for each mode to run a test scenario for.
Comment 1 baerrach 2011-07-06 01:06:40 UTC
Created attachment 27262 [details]
Fix with unit tests
Comment 2 Milamber 2011-07-14 20:15:38 UTC
Thanks for your works.

But, your patch doesn't works very well with the svn trunk. 
There have some errors (when apply patch) with 
* ConstantThroughputTimer.java
* PackageTest.java

I have modified some things to get all patch elements in my eclipse source code.

But now, when I try to run ant tests, I have some errors:
     [java] There were 3 failures:
     [java] 1) testTimer1(org.apache.jmeter.timers.PackageTest)junit.framework.AssertionFailedError: Expected delay of approx 500 expected:<500.0> but was:<2000.0>
     [java] 	at org.apache.jmeter.timers.PackageTest.testTimer1(PackageTest.java:50)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
     [java] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
     [java] 	at org.apache.jorphan.test.AllTests.main(AllTests.java:225)
     [java] 2) testTimer2(org.apache.jmeter.timers.PackageTest)junit.framework.AssertionFailedError: expected:<1> but was:<1001>
     [java] 	at org.apache.jmeter.timers.PackageTest.testTimer2(PackageTest.java:61)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
     [java] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
     [java] 	at org.apache.jorphan.test.AllTests.main(AllTests.java:225)
     [java] 3) testTimer3(org.apache.jmeter.timers.PackageTest)junit.framework.AssertionFailedError: expected:<10> but was:<19>
     [java] 	at org.apache.jmeter.timers.PackageTest.testTimer3(PackageTest.java:74)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
     [java] 	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:39)
     [java] 	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:25)
     [java] 	at org.apache.jorphan.test.AllTests.main(AllTests.java:225)
     [java] FAILURES!!!

Please could you send a new patch (tested it before on a fresh svn trunk & ant tests). Thanks.
Comment 3 baerrach 2011-07-15 07:28:15 UTC
It looks like the patch doesn't apply cleanly on
org.apache.jmeter.timers.ConstantThroughputTimer 
and 
org.apache.jmeter.timers.PackageTest

Dont know why.

resupplying patch and these two files individually.

I also needed to update the build.xml to copy across the simulation files.
Comment 4 baerrach 2011-07-15 07:28:43 UTC
Created attachment 27287 [details]
Trunk Patch
Comment 5 baerrach 2011-07-15 07:29:02 UTC
Created attachment 27288 [details]
ConstantThroughputTimer.java
Comment 6 baerrach 2011-07-15 07:29:20 UTC
Created attachment 27289 [details]
PackageTest.java
Comment 7 Milamber 2011-07-17 10:13:21 UTC
Thanks for new submission. The new patch file is good (including ConstantThroughputTimer and PackageTest).

ant tests works fine now.

I've create a simple test script, mode 4 works fine, but mode 5 don't start.

Message is :
Uncaught Exception java.lang.Error: java.lang.reflect.InvocationTargetException. See log file for details.

Log file :
2011/07/17 10:50:01 ERROR - jmeter.testbeans.TestBeanHelper: This should never happen. java.lang.reflect.InvocationTargetException
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:57)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:616)
	at org.apache.jmeter.testbeans.TestBeanHelper.invokeOrBailOut(TestBeanHelper.java:157)
	at org.apache.jmeter.testbeans.TestBeanHelper.prepare(TestBeanHelper.java:90)
	at org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfStart(StandardJMeterEngine.java:205)
	at org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:341)
	at java.lang.Thread.run(Thread.java:636)
Caused by: java.lang.NullPointerException
	at java.util.concurrent.ConcurrentHashMap.putIfAbsent(ConcurrentHashMap.java:924)
	at org.apache.jmeter.timers.ConstantThroughputTimer.setCalcMode(ConstantThroughputTimer.java:138)
	... 9 more

Uncaught Exception java.lang.Error: java.lang.reflect.InvocationTargetException. See log file for details.
2011/07/17 10:50:01 ERROR - jmeter.JMeter: Uncaught exception:  java.lang.Error: java.lang.reflect.InvocationTargetException
	at org.apache.jmeter.testbeans.TestBeanHelper.invokeOrBailOut(TestBeanHelper.java:166)
	at org.apache.jmeter.testbeans.TestBeanHelper.prepare(TestBeanHelper.java:90)
	at org.apache.jmeter.engine.StandardJMeterEngine.notifyTestListenersOfStart(StandardJMeterEngine.java:205)
	at org.apache.jmeter.engine.StandardJMeterEngine.run(StandardJMeterEngine.java:341)
	at java.lang.Thread.run(Thread.java:636)



Please, for new submission, detab your source code. See:
http://wiki.apache.org/jakarta-jmeter/JMeterEclipse
Comment 8 Milamber 2011-07-17 10:14:33 UTC
Created attachment 27293 [details]
Simple test script

TG3 don't works.
Comment 9 baerrach 2011-07-18 04:31:02 UTC
Looking into it.

As an aside, where is the documentation on how to run JMeter?
The ant task run_gui fails becaose of missing property files ${basedir}/bin/
* jmeter.properties
* saveservice.properties
* upgrade.properties

I've had to copy these in and update the run_gui target to include
      <classpath>
        ...
        <path refid="run.classpath"/>
where
  <path id="run.classpath">
    <fileset dir="${dest.jar}" includes="*.jar"/>
  </path>
Comment 10 baerrach 2011-07-18 04:34:04 UTC
Looks like ConstantThroughputTimer:133
        case CALC_MODE_5_ALL_ACTIVE_THREADS_IN_CURRENT_THREAD_GROUP_SHARED:
            final org.apache.jmeter.threads.AbstractThreadGroup group = JMeterContextService
                    .getContext().getThreadGroup();

is returning null for group
2011/07/18 14:00:37 INFO  - jmeter.timers.ConstantThroughputTimer: group = null 

I didn't notice this, because my real JMeter tests are all CALC_MODE_4_ALL_ACTIVE_THREADS_SHARED

What's the correct way to check is we are in running mode?
Comment 11 baerrach 2011-07-18 05:06:06 UTC
Using
  JMeterContextService.getContext().isSamplingStarted

StandardJMeterEngine.run() seems to set this value when starting.
Comment 12 baerrach 2011-07-18 05:27:04 UTC
ConstantThroughputTimer.setCalcMode becomes:

    /**
     * Setting this has the side effect of sharing <code>throughputInfo</code> if the mode is
     * <em>shared</em> and sampling has started.
     *
     * @param mode
     *            the delay calculation mode
     */
    public void setCalcMode(String mode) {
        this.calcMode = mode;
        // TODO find better way to get modeInt
        this.modeInt = ConstantThroughputTimerBeanInfo.getCalcModeAsInt(calcMode);

        if (JMeterContextService.getContext().isSamplingStarted()) {
            switch (modeInt) {
            case CALC_MODE_4_ALL_ACTIVE_THREADS_SHARED:
                throughputInfo = allThreadsInfo;
                break;

            case CALC_MODE_5_ALL_ACTIVE_THREADS_IN_CURRENT_THREAD_GROUP_SHARED:
                final org.apache.jmeter.threads.AbstractThreadGroup group = JMeterContextService
                        .getContext().getThreadGroup();
                /*
                 * Share the first thread's throughputInfo
                 */
                threadGroupsInfoMap.putIfAbsent(group, throughputInfo);
                throughputInfo = threadGroupsInfoMap.get(group);
                break;
            }
        }
    }

But my ant tests are no longer working...
Comment 13 baerrach 2011-08-10 00:54:50 UTC
Sorry, work has been busy.
Milamber, where did we get to with this?
Comment 14 Graham Russell 2017-11-30 11:37:58 UTC
Is this still an issue?