BIND 10 #2207: define and implement (datasrc::memory::)ZoneUpdater class

[BIND 10 Development]

#2207: define and implement (datasrc::memory::)ZoneUpdater class
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:         |  Sprint-20121023
  medium                             |            Resolution:
                  Component:  data   |             Sensitive:  0
  source                             |           Sub-Project:  DNS
                   Keywords:         |  Estimated Difficulty:  7
            Defect Severity:  N/A    |           Total Hours:  0
Feature Depending on Ticket:         |
  background zone loading            |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei
 * status:  accepted => reviewing


Comment:

 Hello

 I think it is better to assign it to you, because you've already seen part
 of
 the code, while others would have to review from scratch. If you don't
 agree
 and don't want to review it, feel free to put it to unassigned.

 Replying to [comment:6 jinmei]:
 > If you can come up with an idea on how to do it cleanly, please make
 > it happen within this branch (I've not fully thought about it and
 > haven't had a specific idea).  If not, just keep the current design at
 > the moment and defer the further design cleanup to a later phase.

 I don't think the interface itself is problematic. How I'd see it is:
  * With the Local versions, we pass the load_action, it used to load data
 into
    the memory. Just as it is now.
  * In case of the shared memory, the load_action is ignored. The updater
 (now
    renamed to reloader) asks the memory manager for the data by the origin
 and
    class, then waits for confirmation and maps the shared memory to the
 address
    space.
  * In the memory manager, we just reuse the „local“ reloader on top of the
    shared segment.

 Or did I miss something why this couldn't work?

 > - in retrospect `ZoneUpdater` (and file name of zone_updater.xx) might
 >   not be a good choice because it could be confused with
 >   `datasrc::ZoneUpdater`.  If possible, please consider less confusing
 >   name.

 Renamed to ZoneReloader. Is it better?

 > - The latest version of #2268 introduced functions to load a zone into
 >   memory (either from a file or a zone iterator).  It internally
 >   creates a `ZoneData` and returns it with the loaded zone data.
 >   This would be used by the load_action callback, so, instead of
 >   (creating and) passing a `ZoneData` to load_action, we'd simply get
 >   it from these functions.  This means the signature of load_action
 >   should be changed by removing `ZoneData*` from the parameter and
 >   changing the return type from void to `ZoneData*`.  I also think
 >   we should pass `MemorySegment` to load_action.  At the bottom line
 >   these interfaces should be consistent.  So, if you think some
 >   part of these don't make sense, please discuss it with mukund and
 >   make sure these are consistent.

 I decided to just change the interface of LoadAction. It kind of makes
 sense and feels more natural.

 > - In `ZoneUpdaterLocal::install()`, the original intent is to update
 >   the zone table within this method (because this is a local-memory-
 segment
 >   specific detail and we want to hide it from `ConfigurableClientList`).
 >   In the local version, at the moment install_action() is mostly
 >   meaningless, so we could simply pass NULL.

 Thinking about that, I just removed the install_action completely. There's
 no
 need for the local reloader to take it as a parameter on construction and
 then
 not use it. If there's need for one later on in other reloaders, the
 factory
 function (`getZoneReloader`) will just get another parameter.

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