Changeset 2058

Timestamp:

03/03/11 13:12:24 (2 years ago)

Author:

ahu

Message:

move zone freshness checking on incoming notification away from the main query threads and into the slave communicator loop, improving responsiveness when being mass-notified of changes.
Spotted by Richard Poole.

Location:

trunk/pdns/pdns

Files:

• communicator.cc (modified) (2 diffs)
• communicator.hh (modified) (2 diffs)
• dnsbackend.hh (modified) (1 diff)
• packethandler.cc (modified) (3 diffs)
• slavecommunicator.cc (modified) (2 diffs)

trunk/pdns/pdns/communicator.cc

@@ -79 +79 @@
 
     int rc;
-    time_t next;
-
-    time_t tick;
+    time_t next, tick;
 
     for(;;) {
       slaveRefresh(&P);
       masterUpdateCheck(&P);
-      tick=doNotifications();
+      tick=doNotifications(); // this processes any notification acknowledgements and actually send out our own notifications
       
       tick = min (tick, d_tickinterval); 
-
-      //      L<<Logger::Error<<"tick = "<<tick<<", d_tickinterval = "<<d_tickinterval<<endl;
+      
       next=time(0)+tick;
 
@@ -99 +96 @@
           Utility::sleep(1);
         else { 
-          
+          break; // something happened
         }
         // this gets executed at least once every second

trunk/pdns/pdns/communicator.hh

@@ -156 +156 @@
   bool justNotified(const string &domain, const string &ip);
   void addSuckRequest(const string &domain, const string &master, bool priority=false);
+  void addSlaveCheckRequest(const DomainInfo& di, const ComboAddress& remote);
   void notify(const string &domain, const string &ip);
   void mainloop();
@@ -190 +191 @@
   NotificationQueue d_nq;
   bool d_masterschanged, d_slaveschanged;
+  set<DomainInfo> d_tocheck;
 };
 

trunk/pdns/pdns/dnsbackend.hh

@@ -52 +52 @@
   enum {Master,Slave,Native} kind;
   DNSBackend *backend;
+  
+  bool operator<(const DomainInfo& rhs) const
+  {
+    return zone < rhs.zone;
+  }
 };
 

trunk/pdns/pdns/packethandler.cc

@@ -856 +856 @@
   }
     
-  string authServer(p->getRemote());
   if(::arg().contains("trusted-notification-proxy", p->getRemote())) {
     L<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from trusted-notification-proxy "<< p->getRemote()<<endl;
@@ -863 +862 @@
       return RCode::Refused;
     }
-    authServer = *di.masters.begin();
   }
   else if(!db->isMaster(p->qdomain, p->getRemote())) {
@@ -869 +867 @@
     return RCode::Refused;
   }
-  authServer = *di.masters.begin();  // XXX this is actually wrong, we should be picking the master that looks most like the notification!
-  uint32_t theirserial=0;
-
-  Resolver resolver;
-  try {
-    resolver.getSoaSerial(authServer, p->qdomain, &theirserial); // XXX this should have a _really_ short timeout!
-  }
-  catch(ResolverException& re) {
-    L<<Logger::Error<<re.reason<<endl;
-    return RCode::ServFail;
-  }
-
-  if(theirserial<=di.serial) { 
-    L<<Logger::Error<<"Received NOTIFY for "<<p->qdomain<<" from "<< authServer <<", we are up to date: "<<
-      theirserial<<"<="<<di.serial<<endl;
-    return RCode::NoError;
-  }
-  else {
-    L<<Logger::Error<<"Received valid NOTIFY for "<<p->qdomain<<" (id="<<di.id<<") from master "<<p->getRemote()<<": "<<
-      theirserial<<" > "<<di.serial<<endl;
-
-    Communicator.addSuckRequest(p->qdomain, authServer, true); // priority
-    return 0;
-  }
-  return -1; 
+  
+  
+  // ok, we've done our checks
+  Communicator.addSlaveCheckRequest(di, p->remote);
+  
+  return 0;
 }
 

trunk/pdns/pdns/slavecommunicator.cc

@@ -57 +57 @@
   
   if(res.second) {
-  d_suck_sem.post();
+    d_suck_sem.post();
   }
 }
@@ -244 +244 @@
 };
 
+void CommunicatorClass::addSlaveCheckRequest(const DomainInfo& di, const ComboAddress& remote)
+{
+  Lock l(&d_lock);
+  d_tocheck.insert(di);
+  d_any_sem.post(); // kick the loop!
+}
+
 void CommunicatorClass::slaveRefresh(PacketHandler *P)
 {
   UeberBackend *B=dynamic_cast<UeberBackend *>(P->getBackend());
   vector<DomainInfo> rdomains;
-  vector<pair<DomainInfo, bool> > sdomains;
-  B->getUnfreshSlaveInfos(&rdomains);
+  vector<pair<DomainInfo, bool> > sdomains; // the bool is for 'presigned'
+  
+  {
+    Lock l(&d_lock);
+    rdomains.insert(rdomains.end(), d_tocheck.begin(), d_tocheck.end());
+    d_tocheck.clear();
+  }
+  
+  if(rdomains.empty()) // if we have priority domains, check them first
+    B->getUnfreshSlaveInfos(&rdomains);
+    
   DNSSECKeeper dk;
   {