Am 12. Oktober 2016 11:19:08 MESZ, schrieb Philippe Mouawad <philippe.moua...@gmail.com>: >On Wed, Oct 12, 2016 at 11:15 AM, Felix Schumacher < >felix.schumac...@internetallee.de> wrote: > >> >> >> Am 12. Oktober 2016 10:10:30 MESZ, schrieb Philippe Mouawad < >> philippe.moua...@gmail.com>: >> >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.schumac...@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 >> >> I can't see setBodySize(int) in the current source. >> >Ok, I see. 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. >> >> The default values are partly wrong; false instead of 10mb. >> >Are you sure ? >I don't see where . Thx
I don't see it anymore either. On the other hand, the limited storing is slightly wrong. It might store less then the given limit. Felix > >> >> Felix >> >> > >> >> >> >> 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> >> >>>> >> >>>> >> >>>> >> >> >> >>
