Browse Source

Implement retries in forwarder

TODO Merge configuration, merge multiple namespace addresses from
configuration, etc

git-svn-id: svn://bind10.isc.org/svn/bind10/branches/vorner-recursor-timeouts@3415 e5f2f494-b856-4b98-b285-d166d9295462
Michal Vaner 14 years ago
parent
commit
87b73e10d2
3 changed files with 70 additions and 29 deletions
  1. 56 17
      src/lib/asiolink/asiolink.cc
  2. 2 0
      src/lib/asiolink/asiolink.h
  3. 12 12
      src/lib/asiolink/tests/asiolink_unittest.cc

+ 56 - 17
src/lib/asiolink/asiolink.cc

@@ -233,24 +233,63 @@ DNSService::~DNSService() {
 
 RecursiveQuery::RecursiveQuery(DNSService& dns_service, const char& forward,
     uint16_t port, int timeout, unsigned retries) :
-    dns_service_(dns_service), ns_addr_(&forward), port_(port) 
+    dns_service_(dns_service),
+    ns_addr_(&forward),
+    port_(port),
+    timeout_(timeout),
+    retries_(retries)
 {}
 
 namespace {
 
-// This is just temporary so the interface change does not propagate too far
-struct ServerNotify : public UDPQuery::Callback {
-        ServerNotify(DNSServer* server) :
-            server_(server)
-        { }
-        virtual void operator()(UDPQuery::Result result) {
-            server_->resume(result == UDPQuery::SUCCESS);
-            delete this;
-        }
-    private:
-        // FIXME This is said it does problems when it is shared pointer, as
-        // it is destroyed too soon. But who deletes it now?
-        DNSServer* server_;
+class ServerNotify : public UDPQuery::Callback {
+        private:
+            asio::io_service& io_;
+            Question question_;
+            IOAddress address_;
+            uint16_t port_;
+            OutputBufferPtr buffer_;
+            /*
+             * FIXME This is said it does problems when it is shared pointer, as
+             *     it is destroyed too soon. But who deletes it now?
+             */
+            DNSServer* server_;
+            /*
+             * TODO Do something more clever with timeouts. In the long term, some
+             *     computation of average RTT, increase with each retry, etc.
+             */
+            int timeout_;
+            unsigned retries_;
+            void send() {
+                UDPQuery query(io_, question_, address_, port_, buffer_, this,
+                    timeout_);
+                io_.post(query);
+            }
+        public:
+            ServerNotify(asio::io_service& io, const Question &question,
+                const IOAddress& address, uint16_t port,
+                OutputBufferPtr buffer, DNSServer *server, int timeout,
+                unsigned retries) :
+                io_(io),
+                question_(question),
+                address_(address),
+                port_(port),
+                buffer_(buffer),
+                server_(server),
+                timeout_(timeout),
+                retries_(retries)
+            {
+                send();
+            }
+            virtual void operator()(UDPQuery::Result result) {
+                if (result == UDPQuery::TIME_OUT && retries_ --) {
+                    // We timed out, but we have some retries, so send again
+                    send();
+                } else {
+                    server_->resume(result == UDPQuery::SUCCESS);
+                    delete this;
+                }
+            }
 };
 
 }
@@ -265,9 +304,9 @@ RecursiveQuery::sendQuery(const Question& question, OutputBufferPtr buffer,
     // UDP and then fall back to TCP on failure, but for the moment
     // we're only going to handle UDP.
     asio::io_service& io = dns_service_.get_io_service();
-    UDPQuery q(io, question, ns_addr_, port_, buffer,
-        new ServerNotify(server->clone()));
-    io.post(q);
+    // It will delete itself when it is done
+    new ServerNotify(io, question, ns_addr_, port_, buffer, server->clone(),
+         timeout_, retries_);
 }
 
 }

+ 2 - 0
src/lib/asiolink/asiolink.h

@@ -546,6 +546,8 @@ private:
     DNSService& dns_service_;
     IOAddress ns_addr_;
     uint16_t port_;
+    int timeout_;
+    unsigned retries_;
 };
 
 }      // asiolink

+ 12 - 12
src/lib/asiolink/tests/asiolink_unittest.cc

@@ -468,23 +468,21 @@ protected:
     // This version of mock server just stops the io_service when it is resumed
     class MockServerStop : public MockServer {
         public:
-            explicit MockServerStop(asio::io_service& io_service) :
-                MockServer(io_service, asio::ip::address(), 0)
+            explicit MockServerStop(asio::io_service& io_service, bool* done) :
+                MockServer(io_service, asio::ip::address(), 0),
+                done_(done)
             {}
 
-            void operator()(asio::error_code ec = asio::error_code(),
-                            size_t length = 0)
-            {
+            void resume(const bool done) {
+                *done_ = done;
                 io_.stop();
             }
 
             DNSServer* clone() {
                 return (new MockServerStop(*this));
             }
-
-            bool done() const {
-                return done_;
-            }
+        private:
+            bool* done_;
     };
 
 private:
@@ -647,7 +645,7 @@ TEST_F(ASIOLinkTest, recursiveSend) {
 
 void
 receive_and_inc(udp::socket* socket, int* num) {
-    *num ++;
+    (*num) ++;
     static char inbuff[512];
     socket->async_receive(asio::buffer(inbuff, 512),
         boost::bind(receive_and_inc, socket, num));
@@ -669,7 +667,8 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
     receive_and_inc(&socket, &num);
 
     // Prepare the server
-    MockServerStop server(service);
+    bool done(true);
+    MockServerStop server(service, &done);
 
     // Do the answer
     RecursiveQuery query(*dns_service_, *TEST_IPV4_ADDR, port, 10, 2);
@@ -680,7 +679,8 @@ TEST_F(ASIOLinkTest, recursiveTimeout) {
     // Run the test
     service.run();
 
-    EXPECT_TRUE(server.done());
+    // The query should fail
+    EXPECT_FALSE(done);
     EXPECT_EQ(3, num);
 }