Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

vlsi · 2022-09-19T17:23:38Z

Motivation and Context

HeaderManager consumes heap, and it looks it can be reused between threads.

vlsi · 2022-10-29T09:06:14Z

The change would impact the ones who attempt to add/remove headers dynamically via JSR223 or something similar.

For instance, https://sqa.stackexchange.com/questions/41708/jmeter-header-manager-alters-the-headers-during-test attempts

sampler.getHeaderManager().add(new Header("Authorization","Bearer " + vars.get("BEARER")));

vlsi · 2022-10-29T09:41:01Z

I've added if (running version) throw IllegalStateException so the ones who accidentally run into the updating of the shared HTTP header manager would know the limitation.

I'm inclined to merge this as I do not expect it to cause major disruption. At the same time, JMeter's recorder produces header manager for every HTTP sampler, so RAM savings are significant.

vlsi · 2022-10-29T09:59:31Z

Well, it triggers test failures:

java.lang.IllegalStateException: Cannot modify HeaderManager while test is running. If you need dynamic headers, prefer using ${...} expressions in your headers.
	at org.apache.jmeter.protocol.http.control.HeaderManager.assertMutable(HeaderManager.java:63) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.control.HeaderManager.add(HeaderManager.java:171) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.control.HeaderManager.merge(HeaderManager.java:307) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.setHeaderManager(HTTPSamplerBase.java:935) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.addTestElement(HTTPSamplerBase.java:728) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.TestCompiler.configureWithConfigElements(TestCompiler.java:314) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.TestCompiler.configureSampler(TestCompiler.java:102) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.executeSamplePackage(JMeterThread.java:560) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.processSampler(JMeterThread.java:501) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.run(JMeterThread.java:268) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]
2022-10-29 09:48:54,465 ERROR o.a.j.t.JMeterThread: Error while processing sampler: 'HR-BROTLI'.
java.lang.IllegalStateException: Cannot modify HeaderManager while test is running. If you need dynamic headers, prefer using ${...} expressions in your headers.
	at org.apache.jmeter.protocol.http.control.HeaderManager.assertMutable(HeaderManager.java:63) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.control.HeaderManager.add(HeaderManager.java:171) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.control.HeaderManager.merge(HeaderManager.java:307) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.setHeaderManager(HTTPSamplerBase.java:935) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase.addTestElement(HTTPSamplerBase.java:728) ~[ApacheJMeter_http.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.TestCompiler.configureWithConfigElements(TestCompiler.java:314) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.TestCompiler.configureSampler(TestCompiler.java:102) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.executeSamplePackage(JMeterThread.java:560) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.processSampler(JMeterThread.java:501) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at org.apache.jmeter.threads.JMeterThread.run(JMeterThread.java:268) ~[ApacheJMeter_core.jar:5.5.1-SNAPSHOT]
	at java.lang.Thread.run(Thread.java:833) ~[?:?]
:src:dist-check:batchResponseDecompression

FSchumacher · 2022-11-19T11:18:32Z

The change would impact the ones who attempt to add/remove headers dynamically via JSR223 or something similar.

For instance, https://sqa.stackexchange.com/questions/41708/jmeter-header-manager-alters-the-headers-during-test attempts
sampler.getHeaderManager().add(new Header("Authorization","Bearer " + vars.get("BEARER"))); 

Don't you think, this is done more often, than we think about it? I think we have recommended such a workaround for other problems with the samplers setting/not setting headers in the past.

Can we implement a switch to enable sharing of the header manager and expose it with a property (set default to share instances)?

milamberspace · 2023-05-11T07:52:10Z

src/protocol/http/src/main/java/org/apache/jmeter/protocol/http/control/HeaderManager.java

+        if (isRunningVersion()) {
+            throw new IllegalStateException(
+                    "Cannot modify HeaderManager " + getName() + " while test is running. " +
+                            "If you need dynamic headers, prefer using ${...} expressions in your headers."


@vlsi probably better to use JMeter functions than "${...} expressions", because in JMeter's manuel the terms is JMeter functions and variables https://jmeter.apache.org/usermanual/functions.html

Well, the manual says "Functions and Variables". Do you suggest If you need dynamic headers, prefer using ${...} functions or variables in your headers?

However, I think it makes sense to add a common name for "JMeter expression language"

Not sure that the functions in JMeter can be call "expression language", but why not. if you would change to "JMeter expression language", then the manual need to be update (remplace the terms "functions" to "JMeter expression language"), and in JMeter self (menu Tools > Function Helper Dialog)

Function Helper Dialog generates functions. I'm not sure it can generate variables.

expression language

Well, the language is something that declares parsing and semantic rules.
For instance, neither function nor variable can specify if nested ${...} are allowed. Neither function nor variable can specify the way to escape $ if the user needs literal $ value without replacements.

However, in the context of this failure message "function or variable" sounds good enough

pmouawad · 2023-05-11T08:24:48Z

The change would impact the ones who attempt to add/remove headers dynamically via JSR223 or something similar.

For instance, https://sqa.stackexchange.com/questions/41708/jmeter-header-manager-alters-the-headers-during-test attempts
sampler.getHeaderManager().add(new Header("Authorization","Bearer " + vars.get("BEARER"))); 

Hello @vlsi , how would people requiring this feature do then ?
In my performance testing experience I know that for example to be able to test JDEdwards (and another custom product), you need similar code. So if it's not more possible, it's an important problem.

Thanks

pmouawad · 2023-05-11T08:31:55Z

getHeaderManager().add(

I think it's fine through a PreProcessor if you confirm dynamic Headers would still work

…decide if they need cloning or not Previously, components could implements NoThreadClone to prevent cloning in each thread, however, there was no way to "unimplement" the interface. By default, isShareable returns true, so existing components work as before.

…nts by sharing the managers across threads

vlsi · 2023-05-12T12:52:22Z

xdocs/usermanual/properties_reference.xml

@@ -602,6 +602,23 @@ JMETER-SERVER</source>
 </property>
 </properties>
 </section>
+
+  <section name="&sect-num;.14 HTTP Header Manager configuration" anchor="http_header_manager">


Note that &sect-num;.14 duplicates the number of the previous section, however, I believe we'll remove section numbers in #5915

vlsi · 2023-05-12T13:37:55Z

It turns out the case is harder than I thought.
When the script has several HTTP Header Manager components at different levels, then the current JMeter approach is to merge header managers with org.apache.jmeter.protocol.http.control.HeaderManager#merge(org.apache.jmeter.testelement.TestElement)

Apparently, that fails in case HTTP Header Managers are shared across threads.

The next issue is that the "temporary" header managers are not reset after HTTP sampler execution, and they are re-created only before the next execution. In other words, even if we eliminate HTTP Header Manager cloning "before thread starts", then it would generate copies during test execution anyway.

The worst thing is that org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase#getHeaderManager() returns HeaderManager is a public method, so changing it would likely affect backward compatibility.

Frankly speaking, I'm quite disappointed by all that.

vlsi · 2023-05-13T08:15:53Z

Based on the analysis above, I'm moving this PR to the next milestone, so it no longer holds the release.

I wish we could deprecate and drop org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase#getHeaderManager, and org.apache.jmeter.protocol.http.sampler.HTTPSamplerBase#setHeaderManager.

vlsi · 2023-05-13T12:39:14Z

I would push a PR that deprecates the problematic methods. There's a chance the users would notice it and stop using it or ask for a raplacement

See apache#727

vlsi changed the title ~~Add NoThreadClone to HeaderManager to reduce heap utilization~~ Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization Sep 20, 2022

vlsi force-pushed the httpmanager_noclone branch 2 times, most recently from 8b78e35 to cb51d19 Compare October 29, 2022 09:37

vlsi force-pushed the httpmanager_noclone branch from cb51d19 to 1a7ba3f Compare October 29, 2022 10:35

vlsi mentioned this pull request Mar 7, 2023

feat: add property to disable FunctionProperty caching #5788

Merged

vlsi added this to the 5.6 milestone May 10, 2023

milamberspace reviewed May 11, 2023

View reviewed changes

vlsi force-pushed the httpmanager_noclone branch from 1a7ba3f to c280de0 Compare May 12, 2023 05:35

vlsi added 2 commits May 12, 2023 15:36

Save memory when test plan contains lots of HTTP Header Manager eleme…

ad0e8e9

…nts by sharing the managers across threads

vlsi mentioned this pull request May 12, 2023

Add NoThreadClone#isShareable method so the elements can dynamically decide if they need cloning or not #5916

Draft

vlsi force-pushed the httpmanager_noclone branch from c280de0 to ad0e8e9 Compare May 12, 2023 12:39

vlsi commented May 12, 2023

View reviewed changes

vlsi modified the milestones: 5.6, 5.7 May 13, 2023

vlsi marked this pull request as draft May 13, 2023 12:40

vlsi added a commit to vlsi/jmeter that referenced this pull request May 13, 2023

Deprecate getHeaderManager and setHeaderManager in HTTPSamplerBase

f8782c8

See apache#727

vlsi mentioned this pull request May 13, 2023

Deprecate getHeaderManager and setHeaderManager in HTTPSamplerBase #5917

Draft

vlsi added a commit to vlsi/jmeter that referenced this pull request May 13, 2023

Deprecate getHeaderManager and setHeaderManager in HTTPSamplerBase

475e929

See apache#727

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

vlsi commented Sep 19, 2022 •

edited

vlsi commented Oct 29, 2022

vlsi commented Oct 29, 2022

vlsi commented Oct 29, 2022

FSchumacher commented Nov 19, 2022

milamberspace May 11, 2023

vlsi May 11, 2023

milamberspace May 11, 2023

vlsi May 11, 2023

milamberspace May 11, 2023

vlsi May 11, 2023

vlsi May 11, 2023

pmouawad commented May 11, 2023

pmouawad commented May 11, 2023

vlsi May 12, 2023

vlsi commented May 12, 2023

vlsi commented May 13, 2023

vlsi commented May 13, 2023

Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

Are you sure you want to change the base?

Bug 66269: Add NoThreadClone to HeaderManager to reduce heap utilization #727

Conversation

vlsi commented Sep 19, 2022 • edited

Motivation and Context

vlsi commented Oct 29, 2022

vlsi commented Oct 29, 2022

vlsi commented Oct 29, 2022

FSchumacher commented Nov 19, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pmouawad commented May 11, 2023

pmouawad commented May 11, 2023

Choose a reason for hiding this comment

vlsi commented May 12, 2023

vlsi commented May 13, 2023

vlsi commented May 13, 2023

vlsi commented Sep 19, 2022 •

edited