feat: add property to disable FunctionProperty caching #5788
Conversation
|
Frankly speaking, I lean towards changing the default to "do not cache based on iteration number only" as the previous behavior (==cache based on iteration number) was introducing silent and hard-to-notice bugs, so I would alter the default to "do not cache". |
vlsi
force-pushed
the
function_caching
branch
2 times, most recently
from
730dea5 to
fdd8266
Compare
| @@ -102,14 +102,6 @@ public SampleResult sample(Entry e)// Entry tends to be ignored ... | |||
| return res; | |||
| } | |||
| try { | |||
| String request = getScript(); | |||
There was a problem hiding this comment.
@FSchumacher , it turns out this caused two executions of getScript for BeanShellSampler.
One was to pass the script for res.setSamplerData, and the second getScript was in processFileOrScript for actual BSH execution.
I guess that is unexpected. However, the fix does not seem to be trivial: if I change processFileOrScript signature, it breaks backward compatibility because others might have overridden the method.
On the other hand, processFileOrScript does not expose script and fileName, so it is not clear how to get that information out.
For now, I added a new parameter to processFileOrScript so it calls setSamplerData as needed. I think it is unlikely that people override processFileOrScript. I checked GitHub search, and I found no cases for overriding processFileOrScript.
If you are ok with that, I think we need to make the same changes for JSR223 samplers. They have exactly the same issue, and it can call getScript() several times during execution as well.
vlsi
force-pushed
the
function_caching
branch
5 times, most recently
from
33ff2c8 to
62d6e56
Compare
Previously it cached the values based on iteration number only which triggered wrong results on concurrent executions. The previous behavior can be temporary restored with function.cache.per.iteration=true property.
…mpler, and JSR223TestSampler Previously, BeanShellSampler called getScript several times which might cause wrong results when the script included a function that changes on each call (e.g. couter). Now getScript is called only once per sample.
Description
By default, JMeter caches function properties during a test iteration, however, it might cause unexpected results when a component is shared across threads and the expression depends on the thread variables.
It looks like the caching results in wrong HTTP headers being sent if #727 is applied.
Even though JMeter does not modify HTTP Header Manager components during the execution, however, it looks like any function that is used in the headers would be "cached", so other threads could reuse the value generated by a completely different thread.
I believe the current caching behaviour is not well-defined, so I suggest we disable it in the upcoming versions.
The PR does not change the existing behaviour, and it just adds a property so the users could disable caching.