fixed 2 bugs and made clearer that Long-term timer in DnsResolver is not affected...

[pingcheck] / src / host / pingscheduler.cpp

diff --git a/src/host/pingscheduler.cpp b/src/host/pingscheduler.cpp
index 0f08565..b38271a 100644 (file)
--- a/src/host/pingscheduler.cpp
+++ b/src/host/pingscheduler.cpp
@@ -117,7 +117,7 @@ void PingScheduler::stop_pinging()
     // stop pinger and resolver
     GlobalLogger.debug() << LogPrefix << "scheduler: stop pinging";
     Ping->stop_pinging();
-    Resolver->cancel_resolve();
+    cancel_resolve(true);
 
     // now cancel the own timer in case that pinger cancelation called callback
     GlobalLogger.debug() << LogPrefix << "scheduler: cancel timer";
@@ -195,11 +195,9 @@ void PingScheduler::ping_when_ready()
         ip = Resolver->get_next_ip(check_up_to_date);
     }
     if ( ip.is_valid() )
-        Ping->ping(
-                Resolver->get_next_ip().get_ip(),
-                DestinationPort,
-                boost::bind(&PingScheduler::ping_done_handler, this, _1)
-        );
+        Ping->ping( ip.get_ip(),
+                    DestinationPort,
+                    boost::bind(&PingScheduler::ping_done_handler, this, _1) );
     else
     {   // should not happen
         GlobalLogger.error() << LogPrefix << "No IP to ping "
@@ -208,9 +206,6 @@ void PingScheduler::ping_when_ready()
         if ( !Resolver->is_resolving() )
             start_resolving_ping_address();
     }
-
-    // next time try with up-to-date IP
-    ContinueOnOutdatedIps = false;
 }
 
 
@@ -234,6 +229,12 @@ void PingScheduler::ping_done_handler( const bool ping_success )
     update_ping_interval();
     update_ping_elapsed_time();
 
+    if (ping_success)
+    {   // reset ContinueOnOutdatedIps
+        ContinueOnOutdatedIps = false;
+        update_log_prefix();
+    }
+
     // get next protocol, possibly start resolving IPs
     update_ping_protocol();
 
@@ -298,7 +299,7 @@ void PingScheduler::get_next_ping_protocol()
     if (ProtocolIter == Protocols.end())
         ProtocolIter = Protocols.begin();
     PingProtocol ping_protocol = *ProtocolIter;
-    // --> ProtocolIter still points to currently used protocol which is 
+    // --> ProtocolIter still points to currently used protocol which is
     //     required in dns_resolve_callback
 
     if (Ping)
@@ -308,6 +309,7 @@ void PingScheduler::get_next_ping_protocol()
                                        NetworkInterfaceName, PingReplyTimeout);
 
     update_dns_resolver( ping_protocol );
+
 }
 
 bool PingScheduler::can_change_ping_protocol() const
@@ -326,7 +328,7 @@ bool PingScheduler::can_change_ping_protocol() const
 void PingScheduler::update_log_prefix()
 {
     std::stringstream temp;
-    temp << "PS(" << DestinationAddress;
+    temp << "Sched(" << DestinationAddress;
     if (ContinueOnOutdatedIps)
         temp << "!";
     temp << "): ";
@@ -336,10 +338,12 @@ void PingScheduler::update_log_prefix()
 void PingScheduler::update_dns_resolver( PingProtocol current_protocol )
 {
     if (Resolver && Resolver->is_resolving())
+        cancel_resolve(false);
+
+    if (ContinueOnOutdatedIps)
     {
-        GlobalLogger.warning() << LogPrefix
-                          << "Resolver still seems to be resolving --> cancel!";
-        Resolver->cancel_resolve();
+        ContinueOnOutdatedIps = false;
+        update_log_prefix();
     }
 
     // DNS master caches created resolvers and resolved IPs, so this will
@@ -350,7 +354,7 @@ void PingScheduler::update_dns_resolver( PingProtocol current_protocol )
     // start resolving if no ips available
     if ( Resolver->have_up_to_date_ip() )
     {
-        if (!Resolver->is_resolving())
+        if (Resolver->is_resolving())
             GlobalLogger.warning() << LogPrefix << "have up to date IPs but "
                 << "resolver seems to be resolving all the same... "
                 << "Start pinging anyway!";
@@ -373,11 +377,6 @@ void PingScheduler::dns_resolve_callback(const bool was_success,
                         << "with success = " << was_success << " "
                         << "and cname_count = " << cname_count;
 
-    // TODO this is too simple, but need to think more about how to update here!
-    // (may have to switch back some time to resolver for original host or so
-    ContinueOnOutdatedIps = !was_success;
-    update_log_prefix();
-
     if ( was_success )
     {
         HostAnalyzer.set_resolved_ip_count( Resolver->get_resolved_ip_count());
@@ -386,6 +385,8 @@ void PingScheduler::dns_resolve_callback(const bool was_success,
     else
     {   // host name resolution failed; try again bypassing first outdated CNAME
         // or using cached IP
+        ContinueOnOutdatedIps = true;
+        update_log_prefix();
 
         std::string skip_host = Resolver->get_skip_cname();
 
@@ -401,12 +402,43 @@ void PingScheduler::dns_resolve_callback(const bool was_success,
             GlobalLogger.notice() << LogPrefix << "DNS failed, "
                 << "try again skipping a CNAME and resolving "
                 << skip_host << " directly";
+
+            cancel_resolve(false);
+
+            // now create new resolver
             Resolver = DnsMaster::get_instance()
                                    ->get_resolver_for(skip_host, *ProtocolIter);
             start_resolving_ping_address();
-
-            // (the original resolver is still alive and cached by DnsMaster and
-            //  counting down time to re-try on its own until cancel_resolve)
         }
     }
 }
+
+/**
+ * cancel resolver if force_cancel or if it is not resolving DestinationAddress
+ *
+ * Resolvers have a life on their own: they are cached by DnsMaster so never go
+ *   out of scope and even after calling callbacks, there might still be a
+ *   longterm timer active to re-try resolving.
+ * We want to cancel that long-term timer only if the Resolver is not for our
+ *   real, original DestinationAddress but a CNAME, which can happen when trying
+ *   to skip cnames and working on out-dated IPs
+ */
+void PingScheduler::cancel_resolve(const bool force_cancel)
+{
+    if (force_cancel)
+    {
+        GlobalLogger.info() << "Cancelling resolver (forced)";
+        Resolver->cancel_resolve();
+    }
+    else if ( Resolver->get_hostname() == DestinationAddress )
+        GlobalLogger.info() << LogPrefix
+                            << "Leave original resolver active in background";
+    else
+    {
+        GlobalLogger.info() << LogPrefix << "Cancel resolver for "
+            << Resolver->get_hostname() << " since is not the original "
+            << DestinationAddress;
+        Resolver->cancel_resolve();
+    }
+}
+