From: Peter van Dijk <peter.van.dijk@netherlabs.nl>
Date: Tue, 26 Mar 2013 11:44:50 +0000 (+0000)
Subject: unpair name/proto for NS speed management. Fixes #710
X-Git-Tag: rec-3.5-rc3~3
X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=5ea6f7de04ab919fbe145f8d5dd47b89cfbede6f;p=pdns

unpair name/proto for NS speed management. Fixes #710

git-svn-id: svn://svn.powerdns.com/pdns/trunk/pdns@3132 d19b8d6e-7fed-0310-83ef-9ca221ded41b
---

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index e1b23c3eb..1845d775e 100644
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -356,7 +356,7 @@ uint64_t MemRecursorCache::doDumpNSSpeeds(int fd)
   for(SyncRes::nsspeeds_t::iterator i = t_sstorage->nsSpeeds.begin() ; i!= t_sstorage->nsSpeeds.end(); ++i)
   {
     count++;
-    fprintf(fp, "%s|%d -> ", i->first.first.c_str(), i->first.second);
+    fprintf(fp, "%s -> ", i->first.c_str());
     for(SyncRes::DecayingEwmaCollection::collection_t::iterator j = i->second.d_collection.begin(); j!= i->second.d_collection.end(); ++j)
     {
       // typedef vector<pair<ComboAddress, DecayingEwma> > collection_t;
diff --git a/pdns/syncres.cc b/pdns/syncres.cc
index 28f804f7d..69d308846 100644
--- a/pdns/syncres.cc
+++ b/pdns/syncres.cc
@@ -474,7 +474,7 @@ vector<ComboAddress> SyncRes::getAddrs(const string &qname, int type, int depth,
     random_shuffle(ret.begin(), ret.end(), dns_random);
 
     // move 'best' address for this nameserver name up front
-    nsspeeds_t::iterator best = t_sstorage->nsSpeeds.find(make_pair(qname, type));  
+    nsspeeds_t::iterator best = t_sstorage->nsSpeeds.find(qname);
 
     if(best != t_sstorage->nsSpeeds.end())
       for(ret_t::iterator i=ret.begin(); i != ret.end(); ++i) {  
@@ -730,25 +730,23 @@ bool SyncRes::moreSpecificThan(const string& a, const string &b)
 
 struct speedOrder
 {
-  speedOrder(map<SyncRes::typedns_t,double> &speeds) : d_speeds(speeds) {}
-  bool operator()(const SyncRes::typedns_t &a, const SyncRes::typedns_t &b) const
+  speedOrder(map<string,double> &speeds) : d_speeds(speeds) {}
+  bool operator()(const string &a, const string &b) const
   {
     return d_speeds[a] < d_speeds[b];
   }
-  map<SyncRes::typedns_t, double>& d_speeds;
+  map<string, double>& d_speeds;
 };
 
-inline vector<SyncRes::typedns_t> SyncRes::shuffleInSpeedOrder(set<string, CIStringCompare> &tnameservers, const string &prefix)
+inline vector<string> SyncRes::shuffleInSpeedOrder(set<string, CIStringCompare> &tnameservers, const string &prefix)
 {
-  vector<typedns_t> rnameservers;
-  rnameservers.reserve(tnameservers.size()*(1+1*s_doIPv6));
+  vector<string> rnameservers;
+  rnameservers.reserve(tnameservers.size());
   BOOST_FOREACH(const string& str, tnameservers) {
-    rnameservers.push_back(make_pair(str, QType::A));
-    if(s_doIPv6)
-      rnameservers.push_back(make_pair(str, QType::AAAA));
+    rnameservers.push_back(str);
   }
-  map<typedns_t, double> speeds;
-  BOOST_FOREACH(const typedns_t& val, rnameservers) {
+  map<string, double> speeds;
+  BOOST_FOREACH(const string& val, rnameservers) {
     double speed;
     speed=t_sstorage->nsSpeeds[val].get(&d_now);
     speeds[val]=speed;
@@ -759,14 +757,14 @@ inline vector<SyncRes::typedns_t> SyncRes::shuffleInSpeedOrder(set<string, CIStr
   
   if(doLog()) {
     LOG(prefix<<"Nameservers: ");
-    for(vector<typedns_t>::const_iterator i=rnameservers.begin();i!=rnameservers.end();++i) {
+    for(vector<string>::const_iterator i=rnameservers.begin();i!=rnameservers.end();++i) {
       if(i!=rnameservers.begin()) {
         LOG(", ");
         if(!((i-rnameservers.begin())%3)) {
           LOG(endl<<prefix<<"             ");
         }
       }
-      LOG(i->first<<"|"<<((i->second==QType::A) ? "A" : "AAAA") <<"(" << (boost::format("%0.2f") % (speeds[*i]/1000.0)).str() <<"ms)");
+      LOG(*i<<"(" << (boost::format("%0.2f") % (speeds[*i]/1000.0)).str() <<"ms)");
     }
     LOG(endl);
   }
@@ -807,9 +805,9 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
   LOG(prefix<<qname<<": Cache consultations done, have "<<(unsigned int)nameservers.size()<<" NS to contact"<<endl);
 
   for(;;) { // we may get more specific nameservers
-    vector<typedns_t > rnameservers = shuffleInSpeedOrder(nameservers, doLog() ? (prefix+qname+": ") : string() );
+    vector<string > rnameservers = shuffleInSpeedOrder(nameservers, doLog() ? (prefix+qname+": ") : string() );
     
-    for(vector<typedns_t >::const_iterator tns=rnameservers.begin();;++tns) { 
+    for(vector<string >::const_iterator tns=rnameservers.begin();;++tns) { 
       if(tns==rnameservers.end()) {
         LOG(prefix<<qname<<": Failed to resolve via any of the "<<(unsigned int)rnameservers.size()<<" offered NS at level '"<<auth<<"'"<<endl);
         if(auth!="." && flawedNSSet) {
@@ -820,7 +818,7 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
         return -1;
       }
       // this line needs to identify the 'self-resolving' behaviour, but we get it wrong now
-      if(pdns_iequals(qname, tns->first) && qtype.getCode()==QType::A && rnameservers.size() > (unsigned)(1+1*s_doIPv6)) { 
+      if(pdns_iequals(qname, *tns) && qtype.getCode()==QType::A && rnameservers.size() > (unsigned)(1+1*s_doIPv6)) { 
         LOG(prefix<<qname<<": Not using NS to resolve itself!"<<endl);
         continue;
       }
@@ -833,20 +831,20 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
       bool pierceDontQuery=false;
       bool sendRDQuery=false;
       LWResult lwr;
-      if(tns->first.empty()) {
+      if(tns->empty()) {
         LOG(prefix<<qname<<": Domain is out-of-band"<<endl);
         doOOBResolve(qname, qtype, lwr.d_result, depth, lwr.d_rcode);
         lwr.d_tcbit=false;
         lwr.d_aabit=true;
       }
       else {
-        LOG(prefix<<qname<<": Trying to resolve NS '"<<tns->first<<"|"<< (tns->second == QType::A ? "A" : "AAAA") << "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<<endl);
+        LOG(prefix<<qname<<": Trying to resolve NS '"<<*tns<< "' ("<<1+tns-rnameservers.begin()<<"/"<<(unsigned int)rnameservers.size()<<")"<<endl);
 
-        if(!isCanonical(tns->first)) {
+        if(!isCanonical(*tns)) {
           LOG(prefix<<qname<<": Domain has hardcoded nameserver(s)"<<endl);
 
-          string txtAddr = tns->first;
-          if(!tns->first.empty()) {
+          string txtAddr = *tns;
+          if(!tns->empty()) {
             sendRDQuery = txtAddr[0] == '+';
             txtAddr=txtAddr.c_str()+1;
           }
@@ -856,18 +854,18 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
           pierceDontQuery=true;
         }
         else {
-          remoteIPs=getAddrs(tns->first, tns->second, depth+1, beenthere);
+          remoteIPs=getAddrs(*tns, QType::A, depth+1, beenthere);
           pierceDontQuery=false;
         }
 
         if(remoteIPs.empty()) {
-          LOG(prefix<<qname<<": Failed to get "<< (tns->second == QType::A ? "IP" : "IPv6") << " for NS "<<tns->first<<", trying next if available"<<endl);
+          LOG(prefix<<qname<<": Failed to get IP for NS "<<*tns<<", trying next if available"<<endl);
           flawedNSSet=true;
           continue;
         }
         else {
 
-          LOG(prefix<<qname<<": Resolved '"+auth+"' NS "<<tns->first<<" to: ");
+          LOG(prefix<<qname<<": Resolved '"+auth+"' NS "<<*tns<<" to: ");
           for(remoteIP = remoteIPs.begin(); remoteIP != remoteIPs.end(); ++remoteIP) {
             if(remoteIP != remoteIPs.begin()) {
               LOG(", ");
@@ -931,14 +929,14 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
             }
 
             if(lwr.d_rcode==RCode::ServFail || lwr.d_rcode==RCode::Refused) {
-              LOG(prefix<<qname<<": "<<tns->first<<" returned a "<< (lwr.d_rcode==RCode::ServFail ? "ServFail" : "Refused") << ", trying sibling IP or NS"<<endl);
+              LOG(prefix<<qname<<": "<<*tns<<" returned a "<< (lwr.d_rcode==RCode::ServFail ? "ServFail" : "Refused") << ", trying sibling IP or NS"<<endl);
               t_sstorage->throttle.throttle(d_now.tv_sec,make_tuple(*remoteIP, qname, qtype.getCode()),60,3); // servfail or refused
               continue;
             }
             
             break;  // this IP address worked!
           wasLame:; // well, it didn't
-            LOG(prefix<<qname<<": status=NS "<<tns->first<<" ("<< remoteIP->toString() <<") is lame for '"<<auth<<"', trying sibling IP or NS"<<endl);
+            LOG(prefix<<qname<<": status=NS "<<*tns<<" ("<< remoteIP->toString() <<") is lame for '"<<auth<<"', trying sibling IP or NS"<<endl);
             t_sstorage->throttle.throttle(d_now.tv_sec, make_tuple(*remoteIP, qname, qtype.getCode()), 60, 100); // lame
           }
         }
@@ -956,7 +954,7 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
           return RCode::ServFail;
         }
         
-        LOG(prefix<<qname<<": Got "<<(unsigned int)lwr.d_result.size()<<" answers from "<<tns->first<<" ("<< remoteIP->toString() <<"), rcode="<<lwr.d_rcode<<", aa="<<lwr.d_aabit<<", in "<<lwr.d_usec/1000<<"ms"<<endl);
+        LOG(prefix<<qname<<": Got "<<(unsigned int)lwr.d_result.size()<<" answers from "<<*tns<<" ("<< remoteIP->toString() <<"), rcode="<<lwr.d_rcode<<", aa="<<lwr.d_aabit<<", in "<<lwr.d_usec/1000<<"ms"<<endl);
 
         /*  // for you IPv6 fanatics :-)
         if(remoteIP->sin4.sin_family==AF_INET6)
@@ -1131,7 +1129,7 @@ int SyncRes::doResolveAt(set<string, CIStringCompare> nameservers, string auth,
         nameservers=nsset;
         break; 
       }
-      else if(isCanonical(tns->first)) { // means: not OOB (I think)
+      else if(isCanonical(*tns)) { // means: not OOB (I think)
         goto wasLame;
       }
     }
diff --git a/pdns/syncres.hh b/pdns/syncres.hh
index 0204bfd4a..b678ed481 100644
--- a/pdns/syncres.hh
+++ b/pdns/syncres.hh
@@ -309,8 +309,7 @@ public:
     ComboAddress d_best;
   };
 
-  typedef pair<string, uint16_t> typedns_t;
-  typedef map<typedns_t, DecayingEwmaCollection, CIStringPairCompare> nsspeeds_t;  
+  typedef map<string, DecayingEwmaCollection, CIStringCompare> nsspeeds_t;  
 
   struct EDNSStatus
   {
@@ -381,7 +380,7 @@ private:
   string getBestNSNamesFromCache(const string &qname,set<string, CIStringCompare>& nsset, bool* flawedNSSet, int depth, set<GetBestNSAnswer>&beenthere);
   void addAuthorityRecords(const string& qname, vector<DNSResourceRecord>& ret, int depth);
 
-  inline vector<typedns_t> shuffleInSpeedOrder(set<string, CIStringCompare> &nameservers, const string &prefix);
+  inline vector<string> shuffleInSpeedOrder(set<string, CIStringCompare> &nameservers, const string &prefix);
   bool moreSpecificThan(const string& a, const string &b);
   vector<ComboAddress> getAddrs(const string &qname, int type, int depth, set<GetBestNSAnswer>& beenthere);