Add unit test, a bit of documentation and a few small improvements

diff --git a/docs/modes-of-operation.rst b/docs/modes-of-operation.rst
index 867cdc2614b05c671105e550b3e6bf05a092f26a..268f88f5e3d367b0e2e084edafdf8fe14aea71b7 100644 (file)
--- a/docs/modes-of-operation.rst
+++ b/docs/modes-of-operation.rst
@@ -140,6 +140,12 @@ Slave operation can also be programmed using several
 command is especially useful as it triggers an immediate retrieval of
 the zone from the configured master.
 
+Zone transfers are added to a queue and processed according to priority
+and order of addition. Order levels are (from high to low): pdns control,
+api, notify, serial changed during refresh and signatures changed during
+refresh. High priority zone transfers are always processed first, in a 
+first in first out order.
+
 PowerDNS supports multiple masters. For the BIND backend, the native
 BIND configuration language suffices to specify multiple masters, for
 SQL based backends, list all master servers separated by commas in the

diff --git a/pdns/Makefile.am b/pdns/Makefile.am
index 975a12972217450bc74b31914151d95e29179cb3..f497cdddd45ec8315d8529fd932c6e91f2e1da20 100644 (file)
--- a/pdns/Makefile.am
+++ b/pdns/Makefile.am
@@ -1352,6 +1352,7 @@ testrunner_SOURCES = \
        test-base64_cc.cc \
        test-bindparser_cc.cc \
        test-common.hh \
+       test-communicator_hh.cc \
        test-digests_hh.cc \
        test-distributor_hh.cc \
        test-dns_random_hh.cc \

diff --git a/pdns/communicator.cc b/pdns/communicator.cc
index a8fcdacf46de243de5069915815e679d02872f20..45c404aba7fab49c58d621772c6ca38f11d075cb 100644 (file)
--- a/pdns/communicator.cc
+++ b/pdns/communicator.cc
@@ -51,11 +51,10 @@ void CommunicatorClass::retrievalLoopThread()
       if(d_suckdomains.empty()) 
         continue;
 
-      domains_by_queuepos_t& queueindex = boost::multi_index::get<QueueTag>(d_suckdomains);
-      auto firstItem = queueindex.begin();
+      auto firstItem = d_suckdomains.begin();
         
       sr=*firstItem;
-      queueindex.erase(firstItem);
+      d_suckdomains.erase(firstItem);
       if (d_suckdomains.empty()) {
         d_sorthelper = 0;
       }

diff --git a/pdns/communicator.hh b/pdns/communicator.hh
index a8a6a77b012d7f35e192edd356bbfac55318db10..5c993e816a26a70a0643769a7cdb55c573308662 100644 (file)
--- a/pdns/communicator.hh
+++ b/pdns/communicator.hh
@@ -55,17 +55,15 @@ struct SuckRequest
 };
 
 struct IDTag{};
-struct QueueTag{};
 
 typedef multi_index_container<
   SuckRequest,
   indexed_by<
-    ordered_unique<tag<QueueTag>, member<SuckRequest,std::pair<SuckRequest::RequestPriority,uint64_t>,&SuckRequest::priorityAndOrder>>,
+    ordered_unique<member<SuckRequest,std::pair<SuckRequest::RequestPriority,uint64_t>,&SuckRequest::priorityAndOrder>>,
     ordered_unique<tag<IDTag>, identity<SuckRequest> >
   >
 > UniQueue;
 typedef UniQueue::index<IDTag>::type domains_by_name_t;
-typedef UniQueue::index<QueueTag>::type domains_by_queuepos_t;
 
 class NotificationQueue
 {

diff --git a/pdns/test-communicator_hh.cc b/pdns/test-communicator_hh.cc
new file mode 100644 (file)
index 0000000..0973b03
--- /dev/null
+++ b/pdns/test-communicator_hh.cc
@@ -0,0 +1,97 @@
+#define BOOST_TEST_DYN_LINK
+#define BOOST_TEST_NO_MAIN
+#ifdef HAVE_CONFIG_H
+#include "config.h"
+#endif
+#include <stdlib.h>
+#include <unistd.h>
+#include <boost/test/unit_test.hpp>
+#include "communicator.hh"
+
+BOOST_AUTO_TEST_SUITE(test_communicator_hh)
+
+BOOST_AUTO_TEST_CASE(test_axfr_queue_priority_order) {
+  SuckRequest sr[5] = {
+    {DNSName("test1.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::SignaturesRefresh,0}},
+    {DNSName("test2.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::SerialRefresh,1}},
+    {DNSName("test3.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Notify,2}},
+    {DNSName("test4.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,3}},
+    {DNSName("test5.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,4}},
+  };
+
+  UniQueue suckDomains;
+
+  suckDomains.insert(sr[0]);
+  suckDomains.insert(sr[1]);
+  suckDomains.insert(sr[2]);
+  suckDomains.insert(sr[3]);
+  suckDomains.insert(sr[4]);
+
+  for (int i = 4; i >= 0; i--) {
+    auto iter = suckDomains.begin();
+    BOOST_CHECK_EQUAL(iter->domain, sr[i].domain);
+    suckDomains.erase(iter);
+  }
+  BOOST_CHECK(suckDomains.empty());
+}
+
+BOOST_AUTO_TEST_CASE(test_axfr_queue_insert_and_priority_order) {
+  SuckRequest sr[5] = {
+    {DNSName("test1.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,2}},
+    {DNSName("test2.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,1}},
+    {DNSName("test3.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,0}},
+    {DNSName("test4.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,4}},
+    {DNSName("test5.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,3}},
+  };
+
+  UniQueue suckDomains;
+
+  suckDomains.insert(sr[0]);
+  suckDomains.insert(sr[1]);
+  suckDomains.insert(sr[2]);
+  suckDomains.insert(sr[3]);
+  suckDomains.insert(sr[4]);
+
+  for (int i = 4; i >= 0; i--) {
+    auto iter = suckDomains.begin();
+    BOOST_CHECK_EQUAL(iter->domain, sr[i].domain);
+    suckDomains.erase(iter);
+  }
+  BOOST_CHECK(suckDomains.empty());
+}
+
+BOOST_AUTO_TEST_CASE(test_axfr_queue_insert_and_priority_order_after_modify) {
+  SuckRequest sr[5] = {
+    {DNSName("test1.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,1}},
+    {DNSName("test2.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,0}},
+    {DNSName("test3.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::Api,2}},
+    {DNSName("test4.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,4}},
+    {DNSName("test5.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,3}},
+  };
+  SuckRequest rr = {DNSName("test3.com"),ComboAddress("0.0.0.0"),false,{SuckRequest::PdnsControl,5}};
+
+  UniQueue suckDomains;
+
+  suckDomains.insert(sr[0]);
+  suckDomains.insert(sr[1]);
+  suckDomains.insert(sr[2]);
+  suckDomains.insert(sr[3]);
+  suckDomains.insert(sr[4]);
+
+  auto res = suckDomains.insert(rr);
+  BOOST_CHECK(!res.second);
+  suckDomains.modify(res.first, [priorityAndOrder = rr.priorityAndOrder] (SuckRequest& so) {
+    if (priorityAndOrder.first < so.priorityAndOrder.first) {
+      so.priorityAndOrder = priorityAndOrder;
+    }
+  });
+
+  for (int i = 4; i >= 0; i--) {
+    auto iter = suckDomains.begin();
+    BOOST_CHECK_EQUAL(iter->domain, sr[i].domain);
+    suckDomains.erase(iter);
+  }
+  BOOST_CHECK(suckDomains.empty());
+}
+
+BOOST_AUTO_TEST_SUITE_END()