BIND 10 #2211: update the data source reconfigure command so it uses thread

[BIND 10 Development]

#2211: update the data source reconfigure command so it uses thread
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:
  jinmei                             |                Status:  new
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121023
  medium                             |            Resolution:
                  Component:         |             Sensitive:  0
  b10-auth                           |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  6
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Description changed by jinmei:

Old description:

> A subtask of #2201, depends on #2203, #2205, #2210.
>
> This task updates the command handler introduced in #2203 so that it now
> creates a "reconfigure data source" command for the configurator
> thread, and wakes up the thread with it.
>
> Note: according to the `ModuleCCSession` class description, a remote
> module handler is generally expected to be exception free.  Make sure
> the revised handler won't leak exceptions except for real fatal ones.
>
> When this task completes, data source configuration should now be
> possible without disrupting service.
>
> UPDATED: based on the (pre-review) result of #2205 and latest status
> of I'd specifically suggest the following approach (I also assume this
> ticket will be taken before #2213 because this one has fewer
> dependencies).
>
> - as preparation, allow `ModuleCCSession` constructor to take functors
>   (not bare function pointers) for config_handler and command_handler,
>   just like we did for `addRemoteConfig()`.  This is actually a few
>   lines of trivial changes (I know that as I did it experimentally)
> - add something like `AuthSrv::setDataSrcClientsMgr()` and have
>   `AuthSrv` hold a `DataSrcClientsMgr` object.  `AuthSrv` somehow
>   calls `addRemoteConfig()`, whose handler would call something like
>   `DataSrcClientsMgr::reconfigureClients()` (something like this
>   should be introduced in #2210) on the `DataSrcClientsMgr`.
> - also add `AuthSrv::getDataSrcClientsMgr()`, a trivial getter.
> - add an internal `Holder` class to `DataSrcClientsMgr(Base)` class
>   like this:
> {{{#!cpp
> class DataSrcClientsMgrBase {
>     class Holder {
>     public:
>         Holder(DataSrcClientsMgrBase& mgr) : mgr_(mgr),
>                                              locker_(mgr.map_mutex_)
>         {}
>         ConfigurableClientList* getDataSrcClientList(zone_class) {
>             return (mgr_.clients_map_[zone_class].get());
>         }
>     private:
>         DataSrcClientsMgrBase& mgr_;
>         Locker locker_
>
>     };
> };
> }}}
>   The intent of the `Holder` class is to hide the concept of locking
>   from the application.
> - then update the following part of `LoadZoneCommand::exec()`:
> {{{#!cpp
>         isc::util::thread::Mutex::Locker locker(
>             server.getDataSrcClientListMutex());
>         const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
>             list(server.getDataSrcClientList(zone_class));
> }}}
>   as follows:
> {{{#!cpp
>         DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
>         ConfigurableClientList* datasrc_client_list =
>             holder.getDataSrcClientList(zone_class);
> }}}

New description:

 A subtask of #2201, depends on #2203, #2205, #2210.

 This task updates the command handler introduced in #2203 so that it now
 creates a "reconfigure data source" command for the configurator
 thread, and wakes up the thread with it.

 Note: according to the `ModuleCCSession` class description, a remote
 module handler is generally expected to be exception free.  Make sure
 the revised handler won't leak exceptions except for real fatal ones.

 When this task completes, data source configuration should now be
 possible without disrupting service.

 UPDATED: based on the (pre-review) result of #2205 and latest status
 of I'd specifically suggest the following approach (I also assume this
 ticket will be taken before #2213 because this one has fewer
 dependencies).

 - as preparation, allow `ModuleCCSession` constructor to take functors
   (not bare function pointers) for config_handler and command_handler,
   just like we did for `addRemoteConfig()`.  This is actually a few
   lines of trivial changes (I know that as I did it experimentally)
 - add something like `AuthSrv::setDataSrcClientsMgr()` and have
   `AuthSrv` hold a `DataSrcClientsMgr` object.  `AuthSrv` somehow
   calls `addRemoteConfig()`, whose handler would call something like
   `DataSrcClientsMgr::reconfigureClients()` (something like this
   should be introduced in #2210) on the `DataSrcClientsMgr`.
 - also add `AuthSrv::getDataSrcClientsMgr()`, a trivial getter.
 - add an internal `Holder` class to `DataSrcClientsMgr(Base)` class
   like this:
 {{{#!cpp
 class DataSrcClientsMgrBase {
     class Holder {
     public:
         Holder(DataSrcClientsMgrBase& mgr) : mgr_(mgr),
                                              locker_(mgr.map_mutex_)
         {}
         ConfigurableClientList* getDataSrcClientList(zone_class) {
             return (mgr_.clients_map_[zone_class].get());
         }
     private:
         DataSrcClientsMgrBase& mgr_;
         Locker locker_

     };
 };
 }}}
   The intent of the `Holder` class is to hide the concept of locking
   from the application.
 - then update the following part of `LoadZoneCommand::exec()`:
 {{{#!cpp
         isc::util::thread::Mutex::Locker locker(
             server.getDataSrcClientListMutex());
         const boost::shared_ptr<isc::datasrc::ConfigurableClientList>
             list(server.getDataSrcClientList(zone_class));
 }}}
   as follows:
 {{{#!cpp
         DataSrcClientsMgr::Holder holder(server.getDataSrcClientsMgr());
         ConfigurableClientList* datasrc_client_list =
             holder.getDataSrcClientList(zone_class);
 }}}

 Also note: when this ticket is done, we won't have to maintain
 `AuthSrvImpl::datasrc_client_lists`_, and related `AuthSrv` methods.
 They should be cleaned up (and tests should be updated accordingly).

--

-- 
Ticket URL: <http://bind10.isc.org/ticket/2211#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development