apache / apache/jmeter

ConstantThroughputTimer in shared mode all threads start with an incorrect delay of 0

Open
#2,506 14 comments 0 reactions 0 assignees View on GitHub
keyword: PatchAvailable os: All P2
Dominant language
Java
Stars
9.5k
Forks
2.3k
Avg merge
1d 22h
Merged PRs (30d)
5

Description

**baerrach** ([Bug 51480](https://bz.apache.org/bugzilla//show_bug.cgi?id=51480&redirect=false)):
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.

Votes in Bugzilla: 2
Severity: normal
OS: All

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.