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.
