Hello, Maybe we should discuss the default values: httpsampler.max_bytes_to_store_per_request = 10mB => Too high ? To revert to 3.0 behaviour , users can set Integer.MAX_VALUE httpsampler.max_buffer_size = 65 kB => Too high ? To revert to 3.0 behaviour , users can set Integer.MAX_VALUE
Regards Philippe On Wed, Oct 12, 2016 at 10:10 AM, Philippe Mouawad < philippe.moua...@gmail.com> wrote: > Hello, > I commited r1764397 taking into account feedback, at least what I > understood. > Please find details inline below. > > Regards > > On Tue, Oct 11, 2016 at 10:16 PM, Felix Schumacher <felix.schumacher@ > internetallee.de> wrote: > >> Am 11.10.2016 um 21:37 schrieb Philippe Mouawad: >> >>> Hello, >>> Unless there is a nogo, I'll be commiting the patch tomorrow evening. >>> >> Is old public method o.a.j.samplers.SampleResult#setBodySize(int) >> missing after the patch? >> > Fixed > >> >> Javadoc in o.a.j.protocol.http.sampler.HTTPAbstractImpl "Invokes ... >> InputStream, int) ...", shouldn't it be "...InputStream, long)..."? >> > Fixed , please check > >> >> Comment in >> o.a.j.protocol.http.sampler.HTTPFileImpl#MAX_BYTES_TO_STORE_PER_REQUEST >> ... default value: *false*; shouldn't it be 10 MB? >> > Fixed , please check > >> >> Could a o.a.commons.io.input.BoundedInputStream help to shorten our new >> code in o.a.j.protocol.http.sampler.HTTPFileImpl? >> > I don't think so as we need to compute the size of the file even if we > only load part of it. AFAIU, BoundedInputStream does not allow that. > > >> >> Comments in o.a.j.protocol.http.sampler.HTTPSamplerBase same as those in >> HTTPFileImpl. >> > > I didn't understand the problem here. > >> >> Thanks for your work on this, >> Felix >> >>> >>> Regards >>> Philippe >>> >>> On Monday, October 10, 2016, Philippe Mouawad < >>> p.moua...@ubik-ingenierie.com> >>> wrote: >>> >>> Hello , >>>> Any feedback on this ? >>>> I think it should be fixed before next release as it appears for now >>>> that >>>> we cannot handle big downloads. >>>> >>>> Regards >>>> Philippe >>>> >>>> On Sat, Oct 8, 2016 at 9:25 PM, Philippe Mouawad < >>>> p.moua...@ubik-ingenierie.com >>>> <javascript:_e(%7B%7D,'cvml','p.moua...@ubik-ingenierie.com');>> wrote: >>>> >>>> Hello, >>>>> I have attached to BUG 53039 a first patch to handle the bug. >>>>> >>>>> There are several decisions to take regarding this piece of work: >>>>> >>>>> - Introduce a new property that controls how much data from the >>>>> response we store (httpsampler.max_bytes_to_store_per_request). >>>>> Indeed we are limited by array size which is lower than >>>>> Integer.MAX_VALUE >>>>> and even without that, JMeter would not scale if we really save >>>>> the whole >>>>> response. I consider that if response is bigger than a certain >>>>> limit, the >>>>> response is most probably a binary where assertion will be a size >>>>> of a md5 >>>>> hash. >>>>> - Introduce a new property to protect JMeter from big content >>>>> length >>>>> (httpsampler.max_buffer_size). Today we would fail even without >>>>> this issue >>>>> with an OOM due to size of array to allocate. >>>>> - backward compatibility of return methods, I think we need to >>>>> introduce getBytesAsLong and deprecate getBytes(). I'll update >>>>> patch with >>>>> this. >>>>> >>>>> >>>>> >>>>> Regards >>>>> Philippe M. >>>>> >>>>> >>>>> >>>> -- >>>> Cordialement. >>>> Philippe Mouawad. >>>> Ubik-Ingénierie >>>> >>>> UBIK LOAD PACK Web Site <http://www.ubikloadpack.com/> >>>> >>>> UBIK LOAD PACK on TWITTER <https://twitter.com/ubikloadpack> >>>> >>>> >>>> >> > > > -- > Cordialement. > Philippe Mouawad. > > > -- Cordialement. Philippe Mouawad.
