[trac708_test] Added tests for the RTT problem

Added a test to check that the RTT is being calculated correctly.

src/lib/resolve/recursive_query.cc

@@ -57,19 +57,19 @@ RecursiveQuery::RecursiveQuery(DNSService& dns_service,
     const std::vector<std::pair<std::string, uint16_t> >& upstream,
     const std::vector<std::pair<std::string, uint16_t> >& upstream_root,
     int query_timeout, int client_timeout, int lookup_timeout,
-    unsigned retries) :
+    unsigned retries)
+    :
     dns_service_(dns_service),
     nsas_(nsas), cache_(cache),
     upstream_(new AddressVector(upstream)),
     upstream_root_(new AddressVector(upstream_root)),
     test_server_("", 0),
     query_timeout_(query_timeout), client_timeout_(client_timeout),
-    lookup_timeout_(lookup_timeout), retries_(retries)
+    lookup_timeout_(lookup_timeout), retries_(retries), rtt_recorder_()
 {
 }
 
 // Set the test server - only used for unit testing.
-
 void
 RecursiveQuery::setTestServer(const std::string& address, uint16_t port) {
     dlog("Setting test server to " + address + "(" +
@@ -78,6 +78,11 @@ RecursiveQuery::setTestServer(const std::string& address, uint16_t port) {
     test_server_.second = port;
 }
 
+// Set the RTT recorder - only used for testing
+void
+RecursiveQuery::setRttRecorder(boost::shared_ptr<RttRecorder>& recorder) {
+    rtt_recorder_ = recorder;
+}
 
 namespace {
 
@@ -223,6 +228,10 @@ private:
     // event; we can cancel the NSAS callback safely.
     size_t outstanding_events_;
 
+    // RTT Recorder.  Used for testing, the RTTs of queries are
+    // sent to this object as well as being used to update the NSAS.
+    boost::shared_ptr<RttRecorder> rtt_recorder_;
+
     // perform a single lookup; first we check the cache to see
     // if we have a response for our query stored already. if
     // so, call handlerecursiveresponse(), if not, we call send()
@@ -481,7 +490,9 @@ public:
         int query_timeout, int client_timeout, int lookup_timeout,
         unsigned retries,
         isc::nsas::NameserverAddressStore& nsas,
-        isc::cache::ResolverCache& cache) :
+        isc::cache::ResolverCache& cache,
+        boost::shared_ptr<RttRecorder>& recorder)
+        :
         io_(io),
         question_(question),
         answer_message_(answer_message),
@@ -502,7 +513,8 @@ public:
         cur_zone_("."),
         nsas_callback_(new ResolverNSASCallback(this)),
         nsas_callback_out_(false),
-        outstanding_events_(0)
+        outstanding_events_(0),
+        rtt_recorder_(recorder)
     {
         // Setup the timer to stop trying (lookup_timeout)
         if (lookup_timeout >= 0) {
@@ -619,9 +631,11 @@ public:
                 rtt = 1000 * (cur_time.tv_sec - current_ns_qsent_time.tv_sec);
                 rtt += (cur_time.tv_usec - current_ns_qsent_time.tv_usec) / 1000;
             }
-
             dlog("RTT: " + boost::lexical_cast<std::string>(rtt));
             current_ns_address.updateRTT(rtt);
+            if (rtt_recorder_) {
+                rtt_recorder_->addRtt(rtt);
+            }
 
             try {
                 Message incoming(Message::PARSE);
@@ -739,7 +753,8 @@ RecursiveQuery::resolve(const QuestionPtr& question,
             new RunningQuery(io, *question, answer_message, upstream_,
                              test_server_, buffer, callback,
                              query_timeout_, client_timeout_,
-                             lookup_timeout_, retries_, nsas_, cache_);
+                             lookup_timeout_, retries_, nsas_,
+                             cache_, rtt_recorder_);
         }
     }
 }
@@ -792,7 +807,7 @@ RecursiveQuery::resolve(const Question& question,
             new RunningQuery(io, question, answer_message, upstream_,
                              test_server_, buffer, crs, query_timeout_,
                              client_timeout_, lookup_timeout_, retries_,
-                             nsas_, cache_);
+                             nsas_, cache_, rtt_recorder_);
         }
     }
 }

src/lib/resolve/recursive_query.h

@@ -22,11 +22,42 @@
 #include <cache/resolver_cache.h>
 
 namespace asiolink {
-/// \brief The \c RecursiveQuery class provides a layer of abstraction around
-/// the ASIO code that carries out an upstream query.
+
+
+/// \brief RTT Recorder
 ///
-/// This design is very preliminary; currently it is only capable of
-/// handling simple forward requests to a single resolver.
+/// Used for testing, this class will hold the set of round-trip times to
+/// nameservers for the current recursive query.
+///
+/// A pointer to an object of this class is passed to RecursiveQuery which in
+/// turn passes it to the created RunningQuery class.  When a running query
+/// completes, its RTT is passed to the RTT Recorder object.
+class RttRecorder {
+public:
+    /// \brief Record Time
+    ///
+    /// Adds a round-trip time to the internal vector of times.
+    ///
+    /// \param RTT to record.
+    void addRtt(uint32_t rtt) {
+        rtt_.push_back(rtt);
+    }
+
+    /// \brief Return RTT Vector
+    std::vector<uint32_t> getRtt() const {
+        return rtt_;
+    }
+
+private:
+    std::vector<uint32_t>   rtt_;   ///< Stored round-trip times
+};
+
+
+/// \brief Recursive Query
+///
+/// The \c RecursiveQuery class provides a layer of abstraction around
+/// the ASIO code that carries out an upstream query.
+
 class RecursiveQuery {
     ///
     /// \name Constructors
@@ -65,6 +96,14 @@ public:
                    unsigned retries = 3);
     //@}
 
+    /// \brief Set Round-Trip Time Recorder
+    ///
+    /// Sets the RTT recorder object.  This is not accessed directly, instead
+    /// it is passed to created RunningQuery objects.
+    ///
+    /// \param recorder Pointer to the RTT recorder object used to hold RTTs.
+    void setRttRecorder(boost::shared_ptr<RttRecorder>& recorder);
+
     /// \brief Initiate resolving
     /// 
     /// When sendQuery() is called, a (set of) message(s) is sent
@@ -127,6 +166,7 @@ private:
     int client_timeout_;
     int lookup_timeout_;
     unsigned retries_;
+    boost::shared_ptr<RttRecorder>  rtt_recorder_;  ///< Round-trip time recorder
 };
 
 }      // namespace asiolink

src/lib/resolve/tests/recursive_query_unittest_2.cc

@@ -17,6 +17,7 @@
 #include <iomanip>
 #include <iostream>
 #include <string>
+#include <vector>
 
 #include <gtest/gtest.h>
 #include <boost/bind.hpp>
@@ -649,13 +650,17 @@ TEST_F(RecursiveQueryTest2, Resolve) {
                           boost::bind(&RecursiveQueryTest2::tcpAcceptHandler,
                                       this, _1, 0));
 
-    // Set up the RecursiveQuery object.
+    // Set up the RecursiveQuery object. We will also test that it correctly records
+    // RTT times by setting up a RTT recorder object as well.
     std::vector<std::pair<std::string, uint16_t> > upstream;         // Empty
     std::vector<std::pair<std::string, uint16_t> > upstream_root;    // Empty
     RecursiveQuery query(dns_service_, *nsas_, cache_,
                          upstream, upstream_root);
     query.setTestServer(TEST_ADDRESS, TEST_PORT);
 
+    boost::shared_ptr<RttRecorder> recorder(new RttRecorder());
+    query.setRttRecorder(recorder);
+
     // Set up callback to receive notification that the query has completed.
     isc::resolve::ResolverInterface::CallbackPtr
         resolver_callback(new ResolverCallback(service_));
@@ -672,6 +677,16 @@ TEST_F(RecursiveQueryTest2, Resolve) {
     ResolverCallback* rc = static_cast<ResolverCallback*>(resolver_callback.get());
     EXPECT_TRUE(rc->getRun());
     EXPECT_TRUE(rc->getStatus());
+
+    // Finally, check that all the RTTs were "reasonable" (defined here as
+    // being below 2 seconds).  This is an explicit check to test that the
+    // variables in the RTT calculation are at least being initialized; if they
+    // weren't, we would expect some absurdly high answers.
+    vector<uint32_t> rtt = recorder->getRtt();
+    EXPECT_GT(rtt.size(), 0);
+    for (int i = 0; i < rtt.size(); ++i) {
+        EXPECT_LT(rtt[i], 2000);
+    }
 }
 
 } // namespace asiolink