Changeset 1374

Timestamp:

07/28/09 22:33:20 (4 years ago)

Author:

ahu

Message:

clean up recursor cache and make it thread safe, plus change the core logic

Location:

trunk/pdns/pdns

Files:

• recursor_cache.cc (modified) (18 diffs)
• recursor_cache.hh (modified) (6 diffs)

trunk/pdns/pdns/recursor_cache.cc

@@ -6 +6 @@
 #include "arguments.hh"
 #include "syncres.hh"
+#include "lock.hh"
+#include "recursor_cache.hh"
 
 using namespace std;
@@ -12 +14 @@
 #include "config.h"
 
-#ifdef GCC_SKIP_LOCKING
-#include <bits/atomicity.h>
-// This code is ugly but does speedup the recursor tremendously on multi-processor systems, and even has a large effect (20, 30%) on uniprocessor 
-namespace __gnu_cxx
-{
-  _Atomic_word
-  __attribute__ ((__unused__))
-  __exchange_and_add(volatile _Atomic_word* __mem, int __val)
-  {
-    register _Atomic_word __result=*__mem;
-    *__mem+=__val;
-    return __result;
-  }
-
-  void
-  __attribute__ ((__unused__))
-  __atomic_add(volatile _Atomic_word* __mem, int __val)
-  {
-    *__mem+=__val;
-  }
-}
-#endif
-
+pthread_rwlock_t MemRecursorCache::s_rwlock;
 
 DNSResourceRecord String2DNSRR(const string& qname, const QType& qt, const string& serial, uint32_t ttd)
@@ -83 +63 @@
 }
 
+// returns the RDATA for rr - might be compressed!
 string DNSRR2String(const DNSResourceRecord& rr)
 {
@@ -126 +107 @@
 int MemRecursorCache::getDirect(time_t now, const char* qname, const QType& qt, uint32_t ttd[10], char* data[10], uint16_t len[10])
 {
+  ReadLock rl(&s_rwlock);
+
   if(!d_cachecachevalid || Utility::strcasecmp(d_cachedqname.c_str(), qname)) {
 //    cerr<<"had cache cache miss for '"<<qname<<"'"<<endl;
@@ -187 +170 @@
 int MemRecursorCache::get(time_t now, const string &qname, const QType& qt, set<DNSResourceRecord>* res)
 {
+  WriteLock wl(&s_rwlock);
   unsigned int ttd=0;
 
@@ -215 +199 @@
         
         for(vector<StoredRecord>::const_iterator k=i->d_records.begin(); k != i->d_records.end(); ++k) {
-          if(k->d_ttd < 1000000000 || k->d_ttd > (uint32_t) now) {
+          if(k->d_ttd < 1000000000 || k->d_ttd > (uint32_t) now) {  // FIXME what does the 100000000 number mean?
             ttd=k->d_ttd;
             if(res) {
@@ -238 +222 @@
   return -1;
 }
+
+
  
 bool MemRecursorCache::attemptToRefreshNSTTL(const QType& qt, const set<DNSResourceRecord>& content, const CacheEntry& stored)
 {
+  //  WriteLock wl(&s_rwlock); (holds the lock already)
   if(!stored.d_auth) {
 //    cerr<<"feel free to scribble non-auth data!"<<endl;
@@ -272 +259 @@
 void MemRecursorCache::replace(time_t now, const string &qname, const QType& qt,  const set<DNSResourceRecord>& content, bool auth)
 {
+  WriteLock wl(&s_rwlock);
   d_cachecachevalid=false;
   tuple<string, uint16_t> key=make_tuple(qname, qt.getCode());
   cache_t::iterator stored=d_cache.find(key);
-
-  //  cerr<<"storing "<< qname+"|"+qt.getName()<<" -> '"<<content.begin()->content<<"'\n";
 
   bool isNew=false;
@@ -283 +269 @@
     isNew=true;
   }
-  
   pair<vector<StoredRecord>::iterator, vector<StoredRecord>::iterator> range;
 
@@ -289 +274 @@
   CacheEntry ce=*stored;
 
-  if(qt.getCode()==QType::SOA || qt.getCode()==QType::CNAME)  // you can only have one (1) each of these
+  //  cerr<<"storing "<< qname+"|"+qt.getName()<<" -> '"<<content.begin()->content<<"', isnew="<<isNew<<", auth="<<auth<<", ce.auth="<<ce.d_auth<<"\n";
+
+  if(qt.getCode()==QType::SOA || qt.getCode()==QType::CNAME)  { // you can only have one (1) each of these
+    //    cerr<<"\tCleaning out existing store because of SOA and CNAME\n";
     ce.d_records.clear();
-
-  if(auth && !attemptToRefreshNSTTL(qt, content, ce) ) {
-    ce.d_records.clear(); // clear non-auth data
-    ce.d_auth = true;
-    isNew=true;           // data should be sorted again
   }
 
@@ -304 +287 @@
         break;
     if(j != ce.d_records.end()) { // we still have valid data, ignore unauth data
+      //      cerr<<"\tStill hold valid auth data, and the new data is unauth, return\n";
       return;
     }
@@ -310 +294 @@
     }
   }
-
+#if 0
+  if(auth && !attemptToRefreshNSTTL(qt, content, ce) ) {
+    cerr<<"\tGot auth data, and it was not refresh attempt of an NS record, nuking storage"<<endl;
+    ce.d_records.clear(); // clear non-auth data
+    ce.d_auth = true;
+    isNew=true;           // data should be sorted again
+  }
+#endif
+  //  cerr<<"\tHave "<<content.size()<<" records to store\n";
   for(set<DNSResourceRecord>::const_iterator i=content.begin(); i != content.end(); ++i) {
     dr.d_ttd=i->ttl;
@@ -319 +311 @@
     else {
       range=equal_range(ce.d_records.begin(), ce.d_records.end(), dr);
-      
+
+      if(range.first != range.second && (range.first != ce.d_records.begin() || range.second != ce.d_records.end())) {
+        //      cerr<<"\t\tIncomplete match! Must nuke"<<endl;
+        ce.d_records.clear();
+        range.first = range.second = ce.d_records.begin();
+      }
+
       if(range.first != range.second) {
+        //      cerr<<"\t\tMay need to modify TTL of stored record\n";
         for(vector<StoredRecord>::iterator j=range.first ; j!=range.second; ++j) {
           /* see http://mailman.powerdns.com/pipermail/pdns-users/2006-May/003413.html */
-          if(j->d_ttd > (unsigned int) now && i->ttl > j->d_ttd && qt.getCode()==QType::NS && auth) // don't allow auth servers to *raise* TTL of an NS record
+          if(j->d_ttd > (unsigned int) now && i->ttl > j->d_ttd && qt.getCode()==QType::NS && auth) { // don't allow auth servers to *raise* TTL of an NS recor
+            //      cerr<<"\t\tNot doing so, trying to raise TTL NS\n";
             continue;
-          if(i->ttl > j->d_ttd || (auth && d_followRFC2181) ) // authoritative packets can override the TTL to be lower
+          }
+          if(i->ttl > j->d_ttd || (auth && d_followRFC2181) ) { // authoritative packets can override the TTL to be lower
+            //      cerr<<"\t\tUpdating the ttl, diff="<<j->d_ttd - i->ttl<<endl;;
             j->d_ttd=i->ttl;
+          }
+          else {
+            //      cerr<<"\t\tNOT updating the ttl, old= " <<j->d_ttd - now <<", new: "<<i->ttl - now <<endl;
+          }
         }
       }
       else {
+        //      cerr<<"\t\tThere was no exact copy of this record, so adding & sorting\n";
         ce.d_records.push_back(dr);
         sort(ce.d_records.begin(), ce.d_records.end());
@@ -337 +344 @@
 
   if(isNew) {
+    //    cerr<<"\tSorting (because of isNew)\n";
     sort(ce.d_records.begin(), ce.d_records.end());
   }
@@ -348 +356 @@
 int MemRecursorCache::doWipeCache(const string& name, uint16_t qtype)
 {
+  WriteLock wl(&s_rwlock);
   int count=0;
   d_cachecachevalid=false;
@@ -365 +374 @@
 bool MemRecursorCache::doAgeCache(time_t now, const string& name, uint16_t qtype, int32_t newTTL)
 {
+  WriteLock wl(&s_rwlock);
   cache_t::iterator iter = d_cache.find(tie(name, qtype));
   if(iter == d_cache.end()) 
@@ -393 +403 @@
 void MemRecursorCache::doDumpAndClose(int fd)
 {
+  WriteLock wl(&s_rwlock);
   FILE* fp=fdopen(fd, "w");
   if(!fp) {
@@ -424 +435 @@
 void MemRecursorCache::doPrune(void)
 {
+  WriteLock wl(&s_rwlock);
   uint32_t now=(uint32_t)time(0);
   d_cachecachevalid=false;

trunk/pdns/pdns/recursor_cache.hh

@@ -7 +7 @@
 #include "misc.hh"
 #include <iostream>
+
 #include <boost/utility.hpp>
 #undef L
@@ -29 +30 @@
 public:
   MemRecursorCache() : d_followRFC2181(false), d_cachecachevalid(false)
-  {}
+  {
+    pthread_rwlock_init(&s_rwlock, 0);
+    //    cerr<<"ce: "<<sizeof(CacheEntry)<<", sr: "<<sizeof(struct StoredRecord)<<endl;
+  }
   unsigned int size();
   unsigned int bytes();
@@ -63 +67 @@
   };
 
-  struct predicate
-  {
-    predicate(uint32_t limit) : d_limit(limit)
-    {
-    }
-    
-    bool operator()(const StoredRecord& sr) const
-    {
-      return sr.d_ttd <= d_limit;
-    }
-    uint32_t d_limit;
-  };
-
-  //   typedef __gnu_cxx::hash_map<string, vector<StoredRecord> > cache_t;
   struct CacheEntry
   {
-    string d_qname;
-    uint16_t d_qtype;
-    bool d_auth;
-
     CacheEntry(const tuple<string, uint16_t>& key, const vector<StoredRecord>& records, bool auth) : 
       d_qname(key.get<0>()), d_qtype(key.get<1>()), d_auth(auth), d_records(records)
@@ -88 +74 @@
 
     typedef vector<StoredRecord> records_t;
-    records_t d_records;
+
     uint32_t getTTD() const
     {
@@ -99 +85 @@
       return earliest;
     }
+
+    string d_qname;
+    uint16_t d_qtype;
+    bool d_auth;
+    records_t d_records;
   };
 
@@ -122 +113 @@
   bool attemptToRefreshNSTTL(const QType& qt, const set<DNSResourceRecord>& content, const CacheEntry& stored);
 
+  static pthread_rwlock_t s_rwlock;
+
 };
 string DNSRR2String(const DNSResourceRecord& rr);