--------------------- PatchSet 4753 Date: 2007/06/19 05:09:03 Author: rousskov Branch: squid3-icap Tag: (none) Log: Moved OPTIONS fetch time calculation from scheduleUpdate() to optionsFetchTime() and call scheduleUpdate() with the calculated fetch time as a parameter. The change should have no material effect but will help with reviving suspended services. Polished comments and debugging. Members: src/ICAP/ICAPServiceRep.cc:1.1.2.19->1.1.2.20 src/ICAP/ICAPServiceRep.h:1.1.2.13->1.1.2.14 Index: squid3/src/ICAP/ICAPServiceRep.cc =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPServiceRep.cc,v retrieving revision 1.1.2.19 retrieving revision 1.1.2.20 diff -u -r1.1.2.19 -r1.1.2.20 --- squid3/src/ICAP/ICAPServiceRep.cc 1 Jun 2007 21:34:33 -0000 1.1.2.19 +++ squid3/src/ICAP/ICAPServiceRep.cc 19 Jun 2007 05:09:03 -0000 1.1.2.20 @@ -175,8 +175,8 @@ void ICAPServiceRep::noteFailure() { ++theSessionFailures; - debugs(93,4, "ICAPService failure " << theSessionFailures << - ", out of " << TheICAPConfig.service_failure_limit << " allowed"); + debugs(93,4, theSessionFailures << "ICAP service failures, out of " << + TheICAPConfig.service_failure_limit << " allowed " << status()); if (TheICAPConfig.service_failure_limit >= 0 && theSessionFailures > TheICAPConfig.service_failure_limit) @@ -453,7 +453,7 @@ debugs(93,3, "ICAPService got new options and is now " << status()); - scheduleUpdate(); + scheduleUpdate(optionsFetchTime()); scheduleNotification(); } @@ -468,53 +468,62 @@ // Such a timeout should probably be a generic AsyncStart feature. } -void ICAPServiceRep::scheduleUpdate() +void ICAPServiceRep::scheduleUpdate(time_t when) { - if (updateScheduled) - return; // already scheduled - - // XXX: move hard-coded constants from here to TheICAPConfig - - // conservative estimate of how long the OPTIONS transaction will take - const int expectedWait = 20; // seconds - - time_t when = 0; - - if (theOptions && theOptions->valid()) { - const time_t expire = theOptions->expire(); - debugs(93,7, "ICAPService options expire on " << expire << " >= " << squid_curtime); - - // Unknown or invalid (too small) expiration times should not happen. - // ICAPOptions should use the default TTL, and ICAP servers should not - // send invalid TTLs, but bugs and attacks happen. - if (expire < expectedWait) - when = squid_curtime + 60*60; - else - when = expire - expectedWait; // before the current options expire - } else { - // delay for a down service - when = squid_curtime + TheICAPConfig.service_revival_delay; + if (updateScheduled) { + debugs(93,7, "ICAPService reschedules update"); + eventDelete(&ICAPServiceRep_noteTimeToUpdate, this); + updateScheduled = false; } - debugs(93,7, "ICAPService options raw update at " << when << " or in " << + debugs(93,7, HERE << "raw OPTIONS fetch at " << when << " or in " << (when - squid_curtime) << " sec"); + debugs(93,9, HERE << "last fetched at " << theLastUpdate << " or " << + (squid_curtime - theLastUpdate) << " sec ago"); /* adjust update time to prevent too-frequent updates */ if (when < squid_curtime) when = squid_curtime; - const int minUpdateGap = expectedWait + 10; // seconds + // XXX: move hard-coded constants from here to TheICAPConfig + const int minUpdateGap = 30; // seconds if (when < theLastUpdate + minUpdateGap) when = theLastUpdate + minUpdateGap; const int delay = when - squid_curtime; - debugs(93,5, "ICAPService will update options in " << delay << " sec"); + debugs(93,5, "ICAPService will fetch OPTIONS in " << delay << " sec"); + eventAdd("ICAPServiceRep::noteTimeToUpdate", &ICAPServiceRep_noteTimeToUpdate, this, delay, 0, true); updateScheduled = true; } +// returns absolute time when OPTIONS should be fetched +time_t +ICAPServiceRep::optionsFetchTime() const +{ + if (theOptions && theOptions->valid()) { + const time_t expire = theOptions->expire(); + debugs(93,7, "ICAPService options expire on " << expire << " >= " << squid_curtime); + + // conservative estimate of how long the OPTIONS transaction will take + // XXX: move hard-coded constants from here to TheICAPConfig + const int expectedWait = 20; // seconds + + // Unknown or invalid (too small) expiration times should not happen. + // ICAPOptions should use the default TTL, and ICAP servers should not + // send invalid TTLs, but bugs and attacks happen. + if (expire < expectedWait) + return squid_curtime; + else + return expire - expectedWait; // before the current options expire + } + + // use revival delay as "expiration" time for a service w/o valid options + return squid_curtime + TheICAPConfig.service_revival_delay; +} + // returns a temporary string depicting service status, for debugging const char *ICAPServiceRep::status() const { Index: squid3/src/ICAP/ICAPServiceRep.h =================================================================== RCS file: /cvsroot/squid-sf//squid3/src/ICAP/ICAPServiceRep.h,v retrieving revision 1.1.2.13 retrieving revision 1.1.2.14 diff -u -r1.1.2.13 -r1.1.2.14 --- squid3/src/ICAP/ICAPServiceRep.h 1 Jun 2007 21:34:33 -0000 1.1.2.13 +++ squid3/src/ICAP/ICAPServiceRep.h 19 Jun 2007 05:09:03 -0000 1.1.2.14 @@ -1,6 +1,6 @@ /* - * $Id: ICAPServiceRep.h,v 1.1.2.13 2007/06/01 21:34:33 rousskov Exp $ + * $Id: ICAPServiceRep.h,v 1.1.2.14 2007/06/19 05:09:03 rousskov Exp $ * * * SQUID Web Proxy Cache http://www.squid-cache.org/ @@ -47,10 +47,13 @@ host many ICAP services. */ /* - * A service is "up" if there is a fresh cached OPTIONS response and is - * "down" otherwise. A service is "probed" if we tried to get an OPTIONS - * response from it and succeeded or failed. A probed down service is - * called "broken". + * A service with a fresh cached OPTIONS response and without many failures + * is an "up" service. All other services are "down". A service is "probed" + * if we tried to get an OPTIONS response from it and succeeded or failed. + * A probed down service is called "broken". + * + * The number of failures required to bring an up service down is determined + * by icap_service_failure_limit in squid.conf. * * As a bootstrapping mechanism, ICAP transactions wait for an unprobed * service to get a fresh OPTIONS response (see the callWhenReady method). @@ -151,8 +154,9 @@ bool hasOptions() const; bool needNewOptions() const; + time_t optionsFetchTime() const; - void scheduleUpdate(); + void scheduleUpdate(time_t when); void scheduleNotification(); void startGettingOptions();