Hi, I checked , it seems OK regarding multi-threading. Few more notes: MeasuringConnectionManager#dnsResolver is not used
Shouldn't connectTime be added to View Results in Table ? Regards On Sat, Dec 6, 2014 at 2:51 PM, Philippe Mouawad <[email protected] > wrote: > > Hi, > Note there is currently a bugzilla and patch (but partly outdated due to > HttpSampler having been drastically reworked since that time) for this: > > - https://issues.apache.org/bugzilla/show_bug.cgi?id=48799 > > > Regards > > On Sat, Dec 6, 2014 at 2:48 PM, Philippe Mouawad < > [email protected]> wrote: > >> Hi, >> My notes: >> Naming: >> - CSV_CONNECT_TIME , Connect should maybe be named Connection >> - ATT_CONNECT_TIME should maybe be ct (for ConnectTime) instead of cn >> - MeasuringConnectionManager should be >> PoolingClientConnectionManagerAdapter ? >> - MeasuringConnectionRequest should be ClientConnectionRequestAdapter ? >> - MeasuredConnection should be >> MeasuringConnectionManager is missing Apache License header and javadocs >> :-) >> >> public class MeasuringConnectionRequest should not be public static class >> MeasuringConnectionRequest ? >> Same for MeasuredConnection ? >> >> MeasuredConnection : >> connectedTime and startedTime are not used , I suppose it was a work in >> progress code that was not deleted afterwards. >> >> >> More in depth remarks: >> - Are you sure about having SampleResult instance variable of >> MeasuringConnectionManager ? >> This should be checked as I am afraid you may end up sharing SampleResult >> between different samples and threads. >> I need more time to check this. >> >> Regards >> Philippe M. >> @philmdot >> >> >> >> >> On Fri, Dec 5, 2014 at 9:55 PM, Philippe Mouawad < >> [email protected]> wrote: >> >>> Nice, will try to review this we. >>> >>> >>> On Friday, December 5, 2014, Rainer Jung <[email protected]> >>> wrote: >>> >>>> Am 03.12.2014 um 15:15 schrieb Andrey Pokhilko: >>>> >>>>> Happened to be not so much work: >>>>> https://github.com/apache/jmeter/pull/11/files >>>>> >>>>> Please, review it and point me at any changes needed. >>>>> >>>> >>>> I didn't review the patch but I patched a 2.12 I'm currently using to >>>> probe a service and it seems to run well here. >>>> >>>> Regards, >>>> >>>> Rainer >>>> >>>> On 11/29/2014 04:06 PM, sebb wrote: >>>>> >>>>>> On 29 November 2014 at 12:14, Andrey Pokhilko <[email protected]> wrote: >>>>>> >>>>>>> Hi, >>>>>>> >>>>>>> Many times I see a sence to have connect times measured separately, >>>>>>> in >>>>>>> addition to latency that we have in SampleResult. It is important >>>>>>> when >>>>>>> measuring a time for SSL handshake and DNS resolving, when users >>>>>>> want to >>>>>>> see it separate share in total Response Time. >>>>>>> >>>>>>> Connect time is available as separate metric in Grinder and >>>>>>> Yandex.Tank. >>>>>>> The latter has following details on response time pars: connect, >>>>>>> send, >>>>>>> latency, receive. Sometimes some parts are zero, but at least there >>>>>>> is a >>>>>>> technical possibility to see when it is non-zero. It should be noted >>>>>>> that full breakdown would be: dns, connect, send, latency, receive. >>>>>>> >>>>>>> Send and receive times are not of great importance, IMO. And I would >>>>>>> cope with connect time including DNS resolve time. But having connect >>>>>>> time would add interesting aspect on results. >>>>>>> >>>>>> [I expect DNS resolve time might be very tricky to measure in Java] >>>>>> >>>>>> For implementation it will require adding one more property with >>>>>>> getters >>>>>>> and setters to SampleResult, modifying SampleSaveConfiguration and UI >>>>>>> settings to configure saving, using this new field in HTTP sampler, >>>>>>> TCP >>>>>>> sampler, maybe there are other samplers that can respect this field. >>>>>>> >>>>>> The docs would need to be updated to state whether a sampler supports >>>>>> the metric or not. >>>>>> >>>>>> As separate question I would raise if latency should not include >>>>>>> connect >>>>>>> time, for me it sounds logical, but changes existing behavior. >>>>>>> >>>>>> Connect time is currently included in both latency and elapsed. >>>>>> >>>>>> The simplest would be to just add connect as a separate time, but not >>>>>> subtract it from latency or elapsed. >>>>>> This would allow further analysis without changing behaviour. >>>>>> Maybe add an option to perform the subtraction. >>>>>> I don't think we should change the default behaviour. >>>>>> >>>>>> Any opinions? >>>>>>> >>>>>> I can see its use and am not against it, but it needs quite a lot of >>>>>> work to implement fully. >>>>>> >>>>>> -- >>>>>>> Andrey Pokhilko >>>>>>> >>>>>> >>> >>> -- >>> Cordialement. >>> Philippe Mouawad. >>> >>> >>> >>> >> >> >> -- >> Cordialement. >> Philippe Mouawad. >> >> >> > > > -- > Cordialement. > Philippe Mouawad. > > > -- Cordialement. Philippe Mouawad.
