Changeset 658

Timestamp:

04/05/2006 01:11:35 PM (3 years ago)

Author:

ahu

Message:

fix interesting bug whereby powerdns would sometimes claim to be unable to resolve its rootservers. Found by valgrind. Halve number of s_negcache lookups.

Location:

trunk/pdns/pdns

Files:

• syncres.cc (modified) (8 diffs)
• syncres.hh (modified) (2 diffs)

trunk/pdns/pdns/syncres.cc

@@ -66 +66 @@
 int SyncRes::doResolve(const string &qname, const QType &qtype, vector<DNSResourceRecord>&ret, int depth, set<GetBestNSAnswer>& beenthere)
 {
-  
   string prefix;
   if(s_log) {
@@ -73 +72 @@
   }
   
-  int res;
+  int res=0;
   if(!(d_nocache && qtype.getCode()==QType::NS && qname.empty())) {
     if(doCNAMECacheCheck(qname,qtype,ret,depth,res)) // will reroute us if needed
@@ -114 +113 @@
 
   if(!doResolve(qname,QType(QType::A), res,depth+1,beenthere) && !res.empty()) {
-    for(res_t::const_iterator i=res.begin(); i!= res.end(); ++i)
+    for(res_t::const_iterator i=res.begin(); i!= res.end(); ++i) {
       if(i->qtype.getCode()==QType::A) {
         uint32_t ip;
@@ -120 +119 @@
           ret.push_back(ntohl(ip));
       }
+    }
   }
   if(ret.size() > 1)
@@ -248 +248 @@
   QType sqt(qtype);
   uint32_t sttl=0;
-
-  pair<string,QType> tuple(toLower(qname), QType(0));
-  negcache_t::iterator ni=s_negcache.find(tuple);
-
-  if(ni!=s_negcache.end()) {
-    res=0;
-    if((uint32_t)d_now.tv_sec < ni->d_ttd) {
-      sttl=ni->d_ttd - d_now.tv_sec;
-      LOG<<prefix<<qname<<": Entire record '"<<toLower(qname)<<"', is negatively cached for another "<<sttl<<" seconds"<<endl;
-      res=RCode::NXDomain; 
-      giveNegative=true;
-      sqname=ni->d_qname;
-      sqt=QType::SOA;
-    }
-    else {
-      LOG<<prefix<<qname<<": Entire record '"<<toLower(qname)<<"' was negatively cached, but entry expired"<<endl;
-      s_negcache.erase(ni);
-    }
-  }
-
-  if(!giveNegative) { // let's try some more
-    tuple.second=qtype;
-    LOG<<prefix<<qname<<": Looking for direct cache hit of '"<<qname<<"|"<<qtype.getName()<<"', negative cached: "<<s_negcache.count(tuple)<<endl;
-
-    res=0;
-    ni=s_negcache.find(tuple);
-    if(ni!=s_negcache.end()) {
+  //  cout<<"Lookup for '"<<qname<<"|"<<qtype.getName()<<"'\n";
+
+  pair<negcache_t::const_iterator, negcache_t::const_iterator> range=s_negcache.equal_range(tie(toLower(qname)));
+  negcache_t::iterator ni;
+  for(ni=range.first; ni != range.second; ni++) {
+    // we have something
+    if(ni->d_qtype.getCode() == 0 || ni->d_qtype == qtype) {
+      res=0;
       if((uint32_t)d_now.tv_sec < ni->d_ttd) {
         sttl=ni->d_ttd - d_now.tv_sec;
-        LOG<<prefix<<qname<<": "<<qtype.getName()<<" is negatively cached for another "<<sttl<<" seconds"<<endl;
-        res=RCode::NoError; // only this record doesn't exist
+        if(ni->d_qtype.getCode()) {
+          LOG<<prefix<<qname<<": "<<qtype.getName()<<" is negatively cached for another "<<sttl<<" seconds"<<endl;
+          res = RCode::NoError;
+        }
+        else {
+          LOG<<prefix<<qname<<": Entire record '"<<toLower(qname)<<"', is negatively cached for another "<<sttl<<" seconds"<<endl;
+          res= RCode::NXDomain; 
+        }
         giveNegative=true;
         sqname=ni->d_qname;
-        sqt="SOA";
+        sqt=QType::SOA;
+        break;
       }
       else {
-        LOG<<prefix<<qname<<": "<<qtype.getName()<<" was negatively cached, but entry expired"<<endl;
-        s_negcache.erase(ni);
+        LOG<<prefix<<qname<<": Entire record '"<<toLower(qname)<<"' was negatively cached, but entry expired"<<endl;
       }
     }
@@ -315 +302 @@
   
     LOG<<endl;
-    if(found && !expired) 
+    if(found && !expired) {
+      res=0;
       return true;
+    }
     else
       LOG<<prefix<<qname<<": cache had only stale entries"<<endl;
   }
+
   return false;
 }
@@ -527 +517 @@
           ne.d_qname=i->qname;
           ne.d_ttd=d_now.tv_sec + min(i->ttl, 3600U); // controversial
-          ne.d_key.first=toLower(qname);
-          ne.d_key.second=QType(0);
+          ne.d_name=toLower(qname);
+          ne.d_qtype=QType(0);
           s_negcache.insert(ne);
           negindic=true;
@@ -568 +558 @@
           ne.d_qname=i->qname;
           ne.d_ttd=d_now.tv_sec + min(3600U,i->ttl);
-          ne.d_key=make_pair(toLower(qname), qtype);
+          ne.d_name=toLower(qname);
+          ne.d_qtype=qtype;
           if(qtype.getCode())   // prevents us from blacking out a whole domain
             s_negcache.insert(ne);

trunk/pdns/pdns/syncres.hh

@@ -26 +26 @@
 struct NegCacheEntry
 {
-  pair<string, QType> d_key;
+  string d_name;
   QType d_qtype;
   string d_qname;
@@ -249 +249 @@
     indexed_by <
        ordered_unique<
-           member<NegCacheEntry, pair<string,QType>, &NegCacheEntry::d_key>
+           composite_key<
+                 NegCacheEntry,
+                    member<NegCacheEntry, string, &NegCacheEntry::d_name>,
+                    member<NegCacheEntry, QType, &NegCacheEntry::d_qtype>
+           >
        >,
        ordered_non_unique<