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 > > 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> > >>>> > >>>> > >>>> > >> > > -- Cordialement. Philippe Mouawad.
