On Sun, 2021-03-14 at 17:04 -0400, Carter Kozak wrote: > > All right. I will restore 2 second default setting in 5.1 > > Two seconds is a reasonable value, I am supportive of using it. I can > make a case for always executing the check, that's what some other > clients do, but this is more of a brain-dump than an argument for > another value. The 2 second value can make the setting difficult to > discover because it is never reached in synthetic benchmarks, only > when connections are reused just infrequently enough. I would expect > most httpclient5 users aren't substantially impacted by a 1ms delay, > or make requests infrequently enough that they'd be impacted > similarly using the two second timeout. Those who are performance > sensitive likely already have an override in place (I certainly do, > in environments where I can make stronger guarantees). > > I've considered writing an interface to dynamically validate idle > connections, for example I generally don't want to run expensive > validation, but once a connection is discovered to stale I'd like to > require validation for all connections that have been idle longer. > This depends somewhat upon strong support for retries, where however > I don't want to exhaust all available retries when I have a large > number of invalid pooled connections. > > > What other, less expansive technique would you envisage here? > > My understanding is that the nio interface will trigger an event on a > clean remote close, so we may not need to execute the check. I may > very well be incorrect on this point, and it doesn't help if the > remote socket is >
A pluggable interface whose implementations can execute different strategies depending on the actual context makes good sense. One might want to do a relatively cheap socket blocking read when applicable early, or a ping when supported, or a full-blown OPTIONS or a HEAD at a longer idle period, or based on connection quality stats. It can be a pretty cool contribution to the project. Oleg > On Sun, Mar 14, 2021, at 10:38, Oleg Kalnichevski wrote: > > On Sat, 2021-03-13 at 20:23 -0800, Ryan Schmitt wrote: > > > The previous default value of two seconds seemed to work well. > > > > All right. I will restore 2 second default setting in 5.1 > > > > > > > The HTTP/2 > > > client seems to implement staleness checks in an unusually > > > expensive > > > way > > > (sending a PING frame and then waiting for it to return), so it's > > > worth > > > discussing how those two things would interact. > > > > > > > What other, less expansive technique would you envisage here? Ping > > is > > still the cheapest request in HTTP/2 (cheaper than a HEAD or a > > OPTIONS). > > > > Please note that the concept of connection `staleness` in the > > classic > > i/o and the non-blocking i/o models is not the same. Non-blocking > > connections do not become stale just by sitting idle in the > > connection > > pool. Unlike classic connections they can still react to the > > opposite > > endpoint closing the connection on its end. > > > > Oleg > > > > > > > On Sat, Mar 13, 2021 at 12:53 AM Oleg Kalnichevski < > > > [email protected]> > > > wrote: > > > > > > > On Fri, 2021-03-12 at 17:42 -0800, Ryan Schmitt wrote: > > > > > Update: We redeployed the Apache 5 upgrade today with a > > > > > `validateAfterInactivity` setting of one second. Early > > > > > indications so > > > > > far > > > > > are that everything looks good. > > > > > > > > > > I suggest reconsidering the current default setting. The > > > > > upcoming > > > > > 5.1 > > > > > client release is a timely opportunity to make these kinds of > > > > > halftime > > > > > adjustments. > > > > > > > > > > > > > Hi Ryan > > > > > > > > Sure. That makes sense. What default value would you propose? > > > > > > > > Cheers > > > > > > > > Oleg > > > > > > > > > > > > > On Fri, Mar 12, 2021 at 12:45 AM Oleg Kalnichevski < > > > > > [email protected]> > > > > > wrote: > > > > > > > > > > > On Fri, 2021-03-12 at 00:43 -0800, Ryan Schmitt wrote: > > > > > > > OK, I found it. It looks like the default was changed > > > > > > > from > > > > > > > 2000ms > > > > > > > to > > > > > > > null > > > > > > > (disabled) in the move to 5.x. This is a promising line > > > > > > > of > > > > > > > investigation > > > > > > > for me to pursue; I'll report back with what I find. > > > > > > > > > > > > > > > > > > > I was just about to say that. I remember making that change > > > > > > quite > > > > > > early > > > > > > in the 5.0 development phase. This might be it. > > > > > > > > > > > > Oleg > > > > > > > > > > > > > > > > > > > On Fri, Mar 12, 2021 at 12:34 AM Ryan Schmitt < > > > > > > > [email protected]> > > > > > > > wrote: > > > > > > > > > > > > > > > I found changes related to HTTPCLIENT-2094, but it > > > > > > > > seems to > > > > > > > > me > > > > > > > > like > > > > > > > > the > > > > > > > > default value for `validateAfterInactivity` is `null` > > > > > > > > and > > > > > > > > has > > > > > > > > been > > > > > > > > for > > > > > > > > years. Is there something in the classic client (not > > > > > > > > the > > > > > > > > fluent > > > > > > > > client) on > > > > > > > > 4.5.13 that enabled connection validation by default? > > > > > > > > > > > > > > > > On Fri, Mar 12, 2021 at 12:18 AM Ryan Schmitt < > > > > > > > > [email protected]> > > > > > > > > wrote: > > > > > > > > > > > > > > > > > What you're saying about the idle connection > > > > > > > > > validation > > > > > > > > > interval > > > > > > > > > is > > > > > > > > > interesting. Is this the `validateAfterInactivity` > > > > > > > > > setting > > > > > > > > > defined in > > > > > > > > > `PoolingHttpClientConnectionManager`? Are the > > > > > > > > > exceptions > > > > > > > > > caused > > > > > > > > > by the > > > > > > > > > client trying to reuse a connection that's been > > > > > > > > > closed, > > > > > > > > > or > > > > > > > > > half- > > > > > > > > > closed? I > > > > > > > > > had a hunch that something like this could be > > > > > > > > > happening, > > > > > > > > > but > > > > > > > > > I > > > > > > > > > couldn't see > > > > > > > > > how it was possible, particularly for the classic > > > > > > > > > client. > > > > > > > > > (I've > > > > > > > > > only > > > > > > > > > actually seen this sort of issue with async clients.) > > > > > > > > > > > > > > > > > > On Thu, Mar 11, 2021 at 6:58 PM Carter Kozak < > > > > > > > > > [email protected]> > > > > > > > > > wrote: > > > > > > > > > > > > > > > > > > > Is there any way you could reproduce the latency > > > > > > > > > > change > > > > > > > > > > and > > > > > > > > > > failures in > > > > > > > > > > a synthetic environment? If you could share a > > > > > > > > > > reproducer it > > > > > > > > > > would be > > > > > > > > > > incredibly helpful. > > > > > > > > > > > > > > > > > > > > I recall some changes to the idle connection > > > > > > > > > > validation > > > > > > > > > > interval default > > > > > > > > > > value, if the value isn’t set or the feature is > > > > > > > > > > disabled, > > > > > > > > > > connections > > > > > > > > > > closed by remote servers will result in > > > > > > > > > > NoHttpResponseExceptions. Bear in > > > > > > > > > > mind the check it triggers can cost a full > > > > > > > > > > millisecond > > > > > > > > > > on > > > > > > > > > > every > > > > > > > > > > request. > > > > > > > > > > > > > > > > > > > > -Carter > > > > > > > > > > > > > > > > > > > > > On Mar 11, 2021, at 9:45 PM, Gary Gregory < > > > > > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > Also note that 5.1 just hit Maven Central. > > > > > > > > > > > > > > > > > > > > > > Gary > > > > > > > > > > > > > > > > > > > > > > > On Thu, Mar 11, 2021 at 9:39 PM Ryan Schmitt < > > > > > > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > We override (and test) most or all of the > > > > > > > > > > > > timeouts. > > > > > > > > > > > > The > > > > > > > > > > > > calls in > > > > > > > > > > question > > > > > > > > > > > > don't seem to be taking long enough to be > > > > > > > > > > > > getting > > > > > > > > > > > > timed > > > > > > > > > > > > out. > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Mar 11, 2021 at 6:09 PM Gary Gregory > > > > > > > > > > > > > < > > > > > > > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > > Maybe a change in default timeouts from > > > > > > > > > > > > > infinite > > > > > > > > > > > > > to 1 > > > > > > > > > > > > > or > > > > > > > > > > > > > 2 minutes? > > > > > > > > > > > > > > > > > > > > > > > > > > Gary > > > > > > > > > > > > > > > > > > > > > > > > > > On Thu, Mar 11, 2021 at 9:00 PM Ryan Schmitt > > > > > > > > > > > > > < > > > > > > > > > > > > > [email protected]> > > > > > > > > > > wrote: > > > > > > > > > > > > > > On Saturday we rolled out a company-wide > > > > > > > > > > > > > > upgrade > > > > > > > > > > > > > > from > > > > > > > > > > > > > > Apache client > > > > > > > > > > > > > 4.5.13 > > > > > > > > > > > > > > to 5.0.3, and yesterday we ended up rolling > > > > > > > > > > > > > > it > > > > > > > > > > > > > > back > > > > > > > > > > > > > > due > > > > > > > > > > > > > > to several > > > > > > > > > > > > > services > > > > > > > > > > > > > > reporting significant increases in client- > > > > > > > > > > > > > > side > > > > > > > > > > > > > > latency > > > > > > > > > > > > > > and request > > > > > > > > > > > > > failures > > > > > > > > > > > > > > due to NoHttpResponseException. Can someone > > > > > > > > > > > > > > suggest > > > > > > > > > > > > > > a > > > > > > > > > > > > > > good place to > > > > > > > > > > start > > > > > > > > > > > > > > looking for the root cause? > > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------- > > > > > > > > > > > > > ---- > > > > > > > > > > > > > ---- > > > > > > > > > > > > > ---- > > > > > > > > > > > > > -------------- > > > > > > > > > > > > > To unsubscribe, e-mail: > > > > > > > > > > > > > [email protected] > > > > > > > > > > > > > For additional commands, e-mail: > > > > > > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ----------------------------------------------- > > > > > > > > > > > ---- > > > > > > > > > > > ---- > > > > > > > > > > > ---- > > > > > > > > > > > ---------- > > > > > > > > > > > To unsubscribe, e-mail: > > > > > > > > > > > [email protected] > > > > > > > > > > > For additional commands, e-mail: > > > > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > ------------------------------------------------- > > > > > > > > > > ---- > > > > > > > > > > ---- > > > > > > > > > > ---- > > > > > > > > > > -------- > > > > > > > > > > To unsubscribe, e-mail: > > > > > > > > > > [email protected] > > > > > > > > > > For additional commands, e-mail: > > > > > > > > > > [email protected] > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > > --------------------------------------------------------- > > > > > > ---- > > > > > > ---- > > > > > > ---- > > > > > > To unsubscribe, e-mail: [email protected] > > > > > > For additional commands, e-mail: [email protected] > > > > > > > > > > > > > > > > > > > > ------------------------------------------------------------- > > > > ---- > > > > ---- > > > > To unsubscribe, e-mail: [email protected] > > > > For additional commands, e-mail: [email protected] > > > > > > > > > > > > ----------------------------------------------------------------- > > ---- > > To unsubscribe, e-mail: [email protected] > > For additional commands, e-mail: [email protected] > > > > > > --------------------------------------------------------------------- > To unsubscribe, e-mail: [email protected] > For additional commands, e-mail: [email protected] > --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
