On 05/15/2017 08:31 AM, Eduard Bagdasaryan wrote: > I followed your plan with few adjustments and reattached the patch.
LGTM. Will commit soon if there are not objections. Alex. > On 06.05.2017 00:33, Alex Rousskov wrote: >> On 04/27/2017 02:39 PM, Eduard Bagdasaryan wrote: >>> + // always start probing in order to effectively detect >>> + // dead or revived peers >>> + (void)peerProbeConnect(p); >> I think we can simplify that comment while making it more precise: >> >> peerProbeConnect(p); // detect any died or revived peers ASAP >> >> The (void) cast was correct, but is no longer needed after v5 r15133. >> >> The above are minor polishing touches that could be done during commit, >> but I am writing this message because I think we may be able to cover >> one more corner case without significant changes... >> >> Imagine a situation where the peers are already being probed during >> peerDNSConfigure(). Patches Squid will do nothing nothing in this case. >> However, the current probes may be using old/wrong IP addresses (or >> other peer configuration parameters) and, hence, may produce wrong >> result. This lack of a fresh probe may delay the discovery of a died or >> revived peer and might even cause an HTTP transaction failure (in the >> "the peer has recently died" case). >> >> On the other hand, we still do not want to create too many concurrent or >> too frequent probes so we want to continue to honor the current >> peerProbeConnect() concurrency and rate limits. >> >> Please see if the following small change allows Squid to handle the >> above case better: >> >> 1. Add a boolean CachePeer::reprobe field, defaulted to false. It can be >> described as "whether to do another TCP probe after the current TCP >> probes". >> >> 2. peerProbeConnect() should clear the reprobe field after passing the >> two if-statement guards and before entering the loop (even if there are >> no IPs to probe). The right timing here would eliminate any implicit >> reprobing loops while providing the desired functionality. >> >> 3. After closing the connection, peerProbeConnectDone() should call >> peerProbeConnect() if reprobe is true. >> >> 4. Add a boolean reprobeIfBusy parameter to peerProbeConnect() with >> false as the default value. peerDNSConfigure() will call >> peerProbeConnect() with a true parameter value. peerProbeConnect() will >> start by setting reprobe to (reprobe || reprobeIfBusy). >> >> There may be more initialization/reporting/cloning steps needed, just >> like for any other CachePeer data member, but the above reflects the >> core logic. Adjust as needed, of course. >> >> >> Thank you, >> >> Alex. > > > > _______________________________________________ > squid-dev mailing list > squid-dev@lists.squid-cache.org > http://lists.squid-cache.org/listinfo/squid-dev > _______________________________________________ squid-dev mailing list squid-dev@lists.squid-cache.org http://lists.squid-cache.org/listinfo/squid-dev