[2205] overall documentation updates

src/bin/auth/auth_messages.mes

@@ -270,13 +270,34 @@ but has returned an error response (which is included in the message). The
 NOTIFY request will not be honored.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_STARTED data source builder thread started
+A separate thread for maintaining data source clients has been started.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_STOPPED data source builder thread stopped
+The separate thread for maintaining data source clients has been stopped.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_COMMAND data source builder received command, ID: %1
+A debug message, showing when the separate thread for maintaining data
+source clients receives a command from the manager.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_FAILED data source builder thread stopped due to an exception: %1
+The separate thread for maintaining data source clients has been
+terminated due to some uncaught exception.  The manager cannot always
+catch this condition in timely fashion, and there is no way to recover
+from this situation except for restarting the entire server.  So this
+message needs to be carefully watched, and should it occur the auth
+server needs to be restarted by hand.
 
 % AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED data source builder thread stopped due to an unexpected exception
+This is similar to AUTH_DATASRC_CLIENTS_BUILDER_FAILED, but the
+exception type is even more unexpected.  This may rather indicate some
+run time failure than program errors, but in any case the server needs
+to be restarted by hand.
 
 % AUTH_DATASRC_CLIENTS_SHUTDOWN_ERROR error on waiting for data source builder thread: %1
+This indicates that the separate thread for maintaining data source
+clients had been terminated due to an uncaught exception, and the
+manager notices that at its own termination.  There should have been
+AUTH_DATASRC_CLIENTS_BUILDER_FAILED or
+AUTH_DATASRC_CLIENTS_BUILDER_FAILED_UNEXPECTED error messages in past
+logs.  If this message appears, the maintenance of the data source
+clients hasn't been working properly for some time.

src/bin/auth/datasrc_clients_mgr.h

@@ -34,21 +34,77 @@ namespace isc {
 namespace auth {
 
 namespace datasrc_clientmgr_internal {
+// This namespace is essentially private for DataSrcClientsMgr(Base) and
+// DataSrcClientsBuilder(Base).  This is exposed in the public header
+// only because these classes are templated (for testing purposes) and
+// class internal has to be defined here.
+
+/// \brief ID of commands from the DataSrcClientsMgr to DataSrcClientsBuilder.
 enum CommandID {
-    NOOP,                       ///< Do nothing.  Only useful for tests
-    SHUTDOWN                    ///< Shutdown the builder.
+    NOOP,         ///< Do nothing.  Only useful for tests; no argument
+    SHUTDOWN      ///< Shutdown the builder; no argument
 };
+
+/// \brief The data type passed from DataSrcClientsMgr to
+/// DataSrcClientsBuilder.
+///
+/// The first element of the pair is the command ID, and the second element
+/// is its argument.  If the command doesn't take an argument it should be
+/// a null pointer.
 typedef std::pair<CommandID, data::ConstElementPtr> Command;
 } // namespace datasrc_clientmgr_internal
 
+/// \brief Frontend to the manager object for data source clients.
+///
+/// This class provides interfaces for configuring and updating a set of
+/// data source clients "in the background".  The user of this class can
+/// assume any operation on this class can be done effectively non-blocking,
+/// not suspending any delay-sensitive operations such as DNS query
+/// processing.  The only exception is the time when this class object
+/// is destroyed (normally as a result of an implicit call to the destructor);
+/// in the current implementation it can take time depending on what is
+/// running "in the background" at the time of the call.
+///
+/// Internally, an object of this class invokes a separate thread to perform
+/// time consuming operations such as loading large zone data into memory,
+/// but such details are completely hidden from the user of this class.
+///
+/// This class is templated only so that we can test the class without
+/// involving actual threads or mutex.  Normal applications will only
+/// need one specific specialization that has a typedef of
+/// \c DataSrcClientsMgr.
 template <typename ThreadType, typename BuilderType, typename MutexType,
           typename CondVarType>
 class DataSrcClientsMgrBase {
 public:
+    /// \brief Constructor.
+    ///
+    /// It internally invokes a separate thread and waits for further
+    /// operations from the user application.
+    ///
+    /// This method is basically exception free except in case of really
+    /// rare system-level errors.  When that happens the only reasonable
+    /// action that the application can take would be to terminate the program
+    /// in practice.
+    ///
+    /// \throw std::bad_alloc internal memory allocation failure.
+    /// \throw isc::Unexpected general unexpected system errors.
     DataSrcClientsMgrBase() :
         builder_(&command_queue_, &cond_, &queue_mutex_),
         builder_thread_(boost::bind(&BuilderType::run, &builder_))
     {}
+
+    /// \brief The destructor.
+    ///
+    /// It tells the internal thread to stop and waits for it completion.
+    /// In the current implementation, it can block for some unpredictably
+    /// long period depending on what the thread is doing at that time
+    /// (in future we may want to implement a rapid way of killing the thread
+    /// and/or provide a separate interface for waiting so that the application
+    /// can choose the timing).
+    ///
+    /// The waiting operation can result in an exception, but this method
+    /// catches any of them so this method itself is exception free.
     ~DataSrcClientsMgrBase() {
         // We share class member variables with the builder, which will be
         // invalidated after the call to the destructor, so we need to make
@@ -64,6 +120,9 @@ public:
                         data::ConstElementPtr());
             builder_thread_.wait();
         } catch (const util::thread::Thread::UncaughtException& ex) {
+            // technically, logging this could throw, which will be propagated.
+            // But such an exception would be a fatal one anyway, so we
+            // simply let it go through.
             LOG_ERROR(auth_logger, AUTH_DATASRC_CLIENTS_SHUTDOWN_ERROR).
                 arg(ex.what());
         } catch (...) {}
@@ -80,46 +139,80 @@ private:
         cond_.signal();
     }
 
+    //
+    // The following are shared with the builder.
+    //
+    // The list is used as a one-way queue: back-in, front-out
     std::list<datasrc_clientmgr_internal::Command> command_queue_;
-    CondVarType cond_;
-    MutexType queue_mutex_;
+    CondVarType cond_;          // condition variable for queue operations
+    MutexType queue_mutex_;     // mutex to protect the queue
+#ifdef notyet                   // until #2210 or #2212
+    boost::shared_ptr<DataSrcClientListMap> clients_map_;
+    MutexType map_mutex_;
+#endif
+
     BuilderType builder_;
-    ThreadType builder_thread_;
+    ThreadType builder_thread_; // for safety this should be placed last
 };
 
 namespace datasrc_clientmgr_internal {
+
+/// \brief A class that maintains a set of data source clients.
+///
+/// An object of this class is supposed to run on a dedicated thread, whose
+/// main function is a call to its \c run() method.  It runs in a loop
+/// waiting for commands from the manager and handle each command (including
+/// reloading a new version of zone data into memory or fully reconfiguration
+/// of specific set of data source clients.  When it receives a SHUTDOWN
+/// command, it exits from the loop, which will terminate the thread.
+///
+/// This class is a server of \c DataSrcClientsMgr.  Except for tests,
+/// applications should not directly access to this class.
+///
+/// This class is templated so that we can test it without involving actual
+/// threads or locks.
 template <typename MutexType, typename CondVarType>
 class DataSrcClientsBuilderBase {
 public:
+    /// \brief Constructor.
+    ///
+    /// It simply sets up a local copy of shared data with the manager.
+    ///
+    /// Note: this will take actual set (map) of data source clients and
+    /// a mutex object for it in #2210 or #2212.
+    ///
+    /// \throw None
     DataSrcClientsBuilderBase(std::list<Command>* command_queue,
-                              CondVarType* cond, MutexType* queue_mutex) :
+                              CondVarType* cond, MutexType* queue_mutex
+#ifdef notyet
+                              // In #2210 or #2212 we pass other data
+#endif
+        ) :
         command_queue_(command_queue), cond_(cond), queue_mutex_(queue_mutex)
     {}
 
-    /// Not sure if we need this.  It depends on test details.
-    /// \brief Destructor.
-    ///
-    /// This does nothing, but explicitly defined to silence 'unused variable'
-    /// warnings from some versions of clang++.
-    ///~DataSrcClientsBuilderBase() {}
-
+    /// \brief The main loop.
     void run();
 
-    /// separated from run() and made public for the purpose of tests.
+    /// \brief Handle one command from the manager.
+    ///
+    /// This is a dedicated subroutine of run() and is essentially private,
+    /// but is defined as a separate public method so we can test each
+    /// command test individually.  In any case, this class itself is
+    /// generally considered private.
     ///
-    /// \return true if it the builder should keep running; false otherwise.
+    /// \return true if the builder should keep running; false otherwise.
     bool handleCommand(const Command& command);
 
 private:
-    // NOOP command handler.  We use this so tests can override it.
+    // NOOP command handler.  We use this so tests can override it; the default
+    // implementation really does nothing.
     void doNoop() {}
 
-    // end-in, front-out queue
+    // The following are shared with the manager
     std::list<Command>* command_queue_;
     CondVarType* cond_;
     MutexType* queue_mutex_;
-    //boost::shared_ptr<DataSrcClientListMap>* map;
-    //MutexType* data_mutex_;
 };
 
 // Shortcut typedef for normal use

src/bin/auth/tests/test_datasrc_clients_mgr.h

@@ -23,6 +23,14 @@
 
 #include <list>
 
+// In this file we provide specialization of thread, mutex, condition variable,
+// and DataSrcClientsBuilder for convenience of tests.  They don't use
+// actual threads or mutex, and allow tests to inspect some internal states
+// of the corresponding objects.
+//
+// In many cases, tests can use TestDataSrcClientsMgr (defined below) where
+// DataSrcClientsMgr is needed.
+
 // Below we extend the isc::auth::datasrc_clientmgr_internal namespace to
 // specialize the doNoop() method.
 namespace isc {
@@ -54,10 +62,10 @@ public:
     private:
         TestMutex& mutex_;
     };
-    size_t lock_count;
-    size_t unlock_count;
+    size_t lock_count; // number of lock acquisitions; tests can check this
+    size_t unlock_count; // number of lock releases; tests can check this
     size_t noop_count;          // allow doNoop() to modify this
-    ExceptionFromNoop throw_from_noop; // test can set this to control doNoop
+    ExceptionFromNoop throw_from_noop; // tests can set this to control doNoop
 };
 
 class TestCondVar {
@@ -89,8 +97,8 @@ public:
     void signal() {
         ++signal_count;
     }
-    size_t wait_count;
-    size_t signal_count;
+    size_t wait_count; // number of calls to wait(); tests can check this
+    size_t signal_count; // number of calls to signal(); tests can check this
 private:
     std::list<Command>* command_queue_;
     std::list<Command>* delayed_command_queue_;
@@ -145,6 +153,9 @@ public:
     }
 };
 
+// A fake thread class that doesn't really invoke thread but simply calls
+// the given main function (synchronously).  Tests can tweak the wait()
+// behavior via some static variables so it will throw some exceptions.
 class TestThread {
 public:
     TestThread(const boost::function<void()>& main) {