vlsi commented on issue #6330: URL: https://github.com/apache/jmeter/issues/6330#issuecomment-2629532523
@jgaalen , you raise good points, however, please consider the fix would be much faster if you provide a solution, and it would be way faster if the solution includes the relevant tests and documentation. --- This specific issue is caused by Apache HttpClient 4 not properly retrying the requests. We can't do much from JMeter's perspective here. Of course, we could add a retry loop. The issue does not impact the systems I work with, so I'm not that eager to adding a retry or something like that. If you feel confident, you could implement it and suggest a PR. I appreciate you provide tcpdumps, pictures, however, please consider it took significant time to persuade you there's no way to detect and react to FIN in Java code. In the meantime, I asked HttpClient developers about the retry, and they say HttpClient 5 should not be impacted, and they say they are not going to fix HttpClient 4: https://issues.apache.org/jira/browse/HTTPCLIENT-2359 So yet another option for JMeter is to implement HttpClient5-based option for the HTTP Sampler. My wild guess is it should be enough to duplicate `HTTPHC4Impl`, refactor it to use HttpClient5, and add tests (improve existing, and add a few new ones). It does not sound like a dramatic amount of work, however, it would probably require more than a couple of days to polish. At least, if HttpClient 5 won't fix the issue, we could ask HttpClient devs regarding the issue. --- Regarding your PRs, * https://github.com/apache/jmeter/pull/6256 This looks the most trivial one, and it is important since a failed regex is hard to detect indeed. However, the PR misses tests, documentation. * https://github.com/apache/jmeter/pull/6386 I support JMeter should not store past results more than it is necessary, however, it is important to keep backward compatibility. It would like to make sure we don't break scripts that might use post-processors to extract data out of transaction samplers. At the same time, it would be nice to add tests to ensure PR6386 indeed improves memory consumption. * https://github.com/apache/jmeter/pull/6389 Frankly, it does not sound right to add dependency like `SampleResult.setResponseData(byte[] data, String encoding)`. Even though JMeter is often used with HTTP samplers, I'm not sure it is a good idea to hard-code HTTP content encodings into SampleResult. Then there's a question: is there a possibility to add a test case to ensure the fix implements the intended change? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: dev-unsubscr...@jmeter.apache.org For queries about this service, please contact Infrastructure at: us...@infra.apache.org
