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

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

diff --git a/pdns/recursor_cache.cc b/pdns/recursor_cache.cc
index 0c92f127cf5b2157f72e9cd88b05927a58cfdef7..931f0ac99ff9779688791c40786054c487042d6a 100644 (file)
--- a/pdns/recursor_cache.cc
+++ b/pdns/recursor_cache.cc
@@ -5,35 +5,15 @@
 #include "dnsrecords.hh"
 #include "arguments.hh"
 #include "syncres.hh"
+#include "lock.hh"
+#include "recursor_cache.hh"
 
 using namespace std;
 using namespace boost;
 
 #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)
 {
@@ -82,6 +62,7 @@ DNSResourceRecord String2DNSRR(const string& qname, const QType& qt, const strin
   return rr;
 }
 
+// returns the RDATA for rr - might be compressed!
 string DNSRR2String(const DNSResourceRecord& rr)
 {
   uint16_t type=rr.qtype.getCode();
@@ -125,6 +106,8 @@ unsigned int MemRecursorCache::bytes()
 
 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;
     d_cachedqname=qname;
@@ -186,6 +169,7 @@ int MemRecursorCache::getDirect(time_t now, const char* qname, const QType& qt,
 
 int MemRecursorCache::get(time_t now, const string &qname, const QType& qt, set<DNSResourceRecord>* res)
 {
+  WriteLock wl(&s_rwlock);
   unsigned int ttd=0;
 
   //  cerr<<"looking up "<< qname+"|"+qt.getName()<<"\n";
@@ -214,7 +198,7 @@ int MemRecursorCache::get(time_t now, const string &qname, const QType& qt, set<
        sequence_t::iterator si=d_cache.project<1>(i);
        
        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) {
              DNSResourceRecord rr=String2DNSRR(qname, QType(i->d_qtype),  k->d_string, ttd); 
@@ -237,9 +221,12 @@ int MemRecursorCache::get(time_t now, const string &qname, const QType& qt, set<
   }
   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;
     return false;
@@ -271,30 +258,26 @@ bool MemRecursorCache::attemptToRefreshNSTTL(const QType& qt, const set<DNSResou
    touched, but only given a new ttd */
 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;
   if(stored == d_cache.end()) {
     stored=d_cache.insert(CacheEntry(key,vector<StoredRecord>(), auth)).first;
     isNew=true;
   }
-  
   pair<vector<StoredRecord>::iterator, vector<StoredRecord>::iterator> range;
 
   StoredRecord dr;
   CacheEntry ce=*stored;
 
-  if(qt.getCode()==QType::SOA || qt.getCode()==QType::CNAME)  // you can only have one (1) each of these
-    ce.d_records.clear();
+  //  cerr<<"storing "<< qname+"|"+qt.getName()<<" -> '"<<content.begin()->content<<"', isnew="<<isNew<<", auth="<<auth<<", ce.auth="<<ce.d_auth<<"\n";
 
-  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
+  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 && ce.d_auth) {  // unauth data came in, we have some auth data, but is it fresh?
@@ -303,13 +286,22 @@ void MemRecursorCache::replace(time_t now, const string &qname, const QType& qt,
       if((time_t)j->d_ttd > now) 
        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;
     }
     else {
       ce.d_auth = false;  // new data won't be auth
     }
   }
-
+#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;
     dr.d_string=DNSRR2String(*i);
@@ -318,17 +310,32 @@ void MemRecursorCache::replace(time_t now, const string &qname, const QType& qt,
       ce.d_records.push_back(dr);
     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());
       }
@@ -336,6 +343,7 @@ void MemRecursorCache::replace(time_t now, const string &qname, const QType& qt,
   }
 
   if(isNew) {
+    //    cerr<<"\tSorting (because of isNew)\n";
     sort(ce.d_records.begin(), ce.d_records.end());
   }
   
@@ -347,6 +355,7 @@ void MemRecursorCache::replace(time_t now, const string &qname, const QType& qt,
 
 int MemRecursorCache::doWipeCache(const string& name, uint16_t qtype)
 {
+  WriteLock wl(&s_rwlock);
   int count=0;
   d_cachecachevalid=false;
   pair<cache_t::iterator, cache_t::iterator> range;
@@ -364,6 +373,7 @@ int MemRecursorCache::doWipeCache(const string& name, uint16_t qtype)
 
 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()) 
     return false;
@@ -392,6 +402,7 @@ bool MemRecursorCache::doAgeCache(time_t now, const string& name, uint16_t qtype
 
 void MemRecursorCache::doDumpAndClose(int fd)
 {
+  WriteLock wl(&s_rwlock);
   FILE* fp=fdopen(fd, "w");
   if(!fp) {
     close(fd);
@@ -423,6 +434,7 @@ void MemRecursorCache::doSlash(int perc)
 
 void MemRecursorCache::doPrune(void)
 {
+  WriteLock wl(&s_rwlock);
   uint32_t now=(uint32_t)time(0);
   d_cachecachevalid=false;
 

diff --git a/pdns/recursor_cache.hh b/pdns/recursor_cache.hh
index f78daf2882110fe8cc46a095adaa79de34c8ece9..1de0f5226f1833d1bd610cfc394bfe2ef6fbe00e 100644 (file)
--- a/pdns/recursor_cache.hh
+++ b/pdns/recursor_cache.hh
@@ -6,6 +6,7 @@
 #include "qtype.hh"
 #include "misc.hh"
 #include <iostream>
+
 #include <boost/utility.hpp>
 #undef L
 #include <boost/multi_index_container.hpp>
@@ -28,7 +29,10 @@ class MemRecursorCache : public boost::noncopyable //  : public RecursorCache
 {
 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();
   int get(time_t, const string &qname, const QType& qt, set<DNSResourceRecord>* res);
@@ -62,32 +66,14 @@ private:
 
   };
 
-  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)
     {}
 
     typedef vector<StoredRecord> records_t;
-    records_t d_records;
+
     uint32_t getTTD() const
     {
       if(d_records.size()==1)
@@ -98,6 +84,11 @@ private:
        earliest=min(earliest, i->d_ttd);
       return earliest;
     }
+
+    string d_qname;
+    uint16_t d_qtype;
+    bool d_auth;
+    records_t d_records;
   };
 
   typedef multi_index_container<
@@ -121,6 +112,8 @@ private:
   bool d_cachecachevalid;
   bool attemptToRefreshNSTTL(const QType& qt, const set<DNSResourceRecord>& content, const CacheEntry& stored);
 
+  static pthread_rwlock_t s_rwlock;
+
 };
 string DNSRR2String(const DNSResourceRecord& rr);
 DNSResourceRecord String2DNSRR(const string& qname, const QType& qt, const string& serial, uint32_t ttd);