<div dir="ltr"><div><div><div><div><div><div>Hello,<br><br></div>Attached is a patch which makes the changes recommended by Amos - each peer now gets its own event for retrying resolution, dependent on the DNS TTL. This should also fix up the concerns up by Alex. A few caveats though:<br><br></div><div> - the cache manager shows generic "peerRefreshDNS" names for each event. I can't find any examples that give it a dynamic name, e.g. I'd like something like "peerRefreshDNS(<a href="http://example.com">example.com</a>)", but I can't think of how I'd do that without leaking memory or making some significant changes to the event handler system.<br><br></div><div>- I can't figure out how to reproduce the second failure case, where a result comes back but it has no IP addresses. I _think_ using the TTL would be valid instead of negative_dns_ttl would be valid in that situation, but I can't be sure. I figured this was the safest option.<br></div><div><br></div> - eventDelete does not appear to be clearing out events as I expect it to, so if you reconfigure Squid you end up with some dead events, like so:<br><br></div>    [root@xxx ~]# squidmgr events | grep peerRefresh<br>Last event to run: peerRefreshDNS<br>peerRefreshDNS                  0.331 sec           1    yes<br>peerRefreshDNS                  0.679 sec           1    yes<br>peerRefreshDNS                  47.649 sec          1    yes<br>peerRefreshDNS                  61.619 sec          1    yes<br>peerRefreshDNS                  207.682 sec         1    yes<br>peerRefreshDNS                  207.682 sec         1    yes<br>peerRefreshDNS                  207.682 sec         1    yes<br>peerRefreshDNS                  207.682 sec         1    yes<br>peerRefreshDNS                  207.682 sec         1    yes<br>[root@xxx ~]# squid -k reconfigure                                                                <br>[root@xxx ~]# squidmgr events | grep peerRefresh<br>Last event to run: peerRefreshDNS<br>peerRefreshDNS                  0.763 sec           1    yes<br>peerRefreshDNS                  0.763 sec           1    yes<br>peerRefreshDNS                  41.755 sec          1    yes<br>peerRefreshDNS                  55.755 sec          1    yes<br>peerRefreshDNS                  56.187 sec          1    no<br>peerRefreshDNS                  202.250 sec         1    no<br>peerRefreshDNS                  202.250 sec         1    no<br>peerRefreshDNS                  3599.758 sec        1    yes<br>peerRefreshDNS                  3599.758 sec        1    yes<br>peerRefreshDNS                  3599.758 sec        1    yes<br>peerRefreshDNS                  3599.758 sec        1    yes<br>peerRefreshDNS                  3599.758 sec        1    yes<br><br></div>If I run squid -k reconfigure again, then the events with invalid callback data are cleared out, so it doesn't grow indefinitely at least. I'm not sure how or if I should fix this.<br><br></div>Thank you,<br><br></div>Nathan.<br><div><div><div><br></div></div></div></div><div class="gmail_extra"><br><div class="gmail_quote">On 10 May 2016 at 18:13, Alex Rousskov <span dir="ltr"><<a href="mailto:rousskov@measurement-factory.com" target="_blank">rousskov@measurement-factory.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span class="">On 05/10/2016 01:50 AM, Amos Jeffries wrote:<br>
<br>
> Then each peer gets its own re-lookup event scheduled<br>
<br>
</span>If applied correctly, this approach would also solve the misapplication<br>
problem I described in my concurrent review. Unfortunately, it requires<br>
serious work. Fortunately, you have already converted CachePeer from<br>
being a POD into a proper class. That will help!<br>
<br>
<br>
Thank you,<br>
<br>
Alex.<br>
<br>
</blockquote></div><br></div>