jmeter icon indicating copy to clipboard operation
jmeter copied to clipboard

65041: httpclient.timeout property not working as expected

Open mjmadhu opened this issue 4 years ago • 4 comments

Description

Support to http.socket.timeout through properties

  • read httpclient parameters from hc.parameters.file if provided in jmeter.properties
  • set http socket timeout for HTTP Sampler in the following preference order:
      1. Response timeout in HTTP Sampler/ HTTP Request Defaults
      1. http.socket.timeout in hc.parameters
      1. httpclient.timeout property in jmeter.properties

Motivation and Context

  • JMeter tests were not stopping in distributed mode in our system. We got thread dumps and found forever hanging HTTP request threads to be the main cause for the issue. Since we're not the author of the test plans, we can't assure timeouts to be always configured in test plan. Hence, we require some jmeter properties to control this behavior, only to find they were not working as expected.
  • Logged the following bug: https://bz.apache.org/bugzilla/show_bug.cgi?id=65041

How Has This Been Tested?

  • Added JUnit tests and verified them to be passed.
  • Verified JMeter to be built and working fine.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)

Checklist:

  • [x] My code follows the code style of this project.
  • [ ] I have updated the documentation accordingly.

mjmadhu avatar Jan 05 '21 13:01 mjmadhu

Thanks for the PR. In https://lists.apache.org/thread.html/r336aebd7963f0d377291cdd58aa9e74b6427a3e888da00441d78f6aa%40%3Cdev.jmeter.apache.org%3E Philippe has tried another way of implementing this, which was hindered by a small bug. I think that bug should be fixed. What do you think?

FSchumacher avatar Feb 11 '21 20:02 FSchumacher

Thanks for the PR. In https://lists.apache.org/thread.html/r336aebd7963f0d377291cdd58aa9e74b6427a3e888da00441d78f6aa%40%3Cdev.jmeter.apache.org%3E Philippe has tried another way of implementing this, which was hindered by a small bug. I think that bug should be fixed. What do you think?

Hi, I think there's a difference in approach. The intent here is also to support timeout values provided via jmeter properties and hc.parameters files given by the end users. The final HTTP request made from HTTPHCAbstractImpl.java will pick a response timeout from the 3 options in the following order of precedence (Response timeout in HTTP Sampler/ HTTP Request Defaults > http.socket.timeout in hc.parameters > httpclient.timeout property in jmeter.properties) - https://github.com/apache/jmeter/pull/640/files#diff-29609bda5c038501a23cff12e470709027f303766ea1f787c6d69dd71dce8819R199

It's different from https://lists.apache.org/thread.html/r336aebd7963f0d377291cdd58aa9e74b6427a3e888da00441d78f6aa%40%3Cdev.jmeter.apache.org%3E as:

  1. We're not trying to change/set default timeout value "only" via HTTP sampler. Hence, we did not hit that bug as default value is still 0 if not provided.
  2. This PR is only for response timeout as of now, not including connect timeout.

If we want to give specific default values for these timeouts, we can still have them in jmeter.properties with this feature support.

mjmadhu avatar Feb 12 '21 11:02 mjmadhu

@FSchumacher Hi, Is there anything else we can help with for merging this PR?

mjmadhu avatar Jul 22 '21 04:07 mjmadhu

@ubikloadpack Apologies for delay. I missed pushing the changes somehow. Please review.

mjmadhu avatar Jan 05 '22 05:01 mjmadhu