BIND 10 #2854: memory manager framework

[BIND 10 Development]

#2854: memory manager framework
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  jinmei
            Priority:  medium        |                       Status:
           Component:  shmem         |  reviewing
  manager                            |                    Milestone:
            Keywords:                |  Sprint-20130625
           Sensitive:  0             |                   Resolution:
         Sub-Project:  DNS           |                 CVSS Scoring:
Estimated Difficulty:  9             |              Defect Severity:  N/A
         Total Hours:  0             |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 I think a changelog would be nice, even if the daemon is not of any use.
 The user might get interested in the fact that a new program exists, so we
 should say he should leave it alone. Also, we should update the component
 status wiki page with the new daemon.

 The tests fail for me:
 {{{
 Running test: datasrc_info_tests.py

 .
 E
 .
 .
 .
 .

 ======================================================================
 ERROR: test_production (__main__.TestDataSrcInfo)
 Check the behavior closer to a production environment.
 ----------------------------------------------------------------------
 Traceback (most recent call last):
   File
 "/tmp/bind10/bind10-2/bind10-20130529/_build/../src/lib/python/isc/memmgr/tests/datasrc_info_tests.py",
 line 171, in test_production
     cmgr.reconfigure(datasrc_config)
 TypeError: reconfigure() missing 1 required positional argument:
 'config_data'

 ----------------------------------------------------------------------
 Ran 6 tests in 0.001s
 }}}

 And, there's bunch of smaller things.

 Are these tabs?
 {{{
                   src/bin/loadzone/tests/correct/Makefile
 +                src/bin/memmgr/Makefile
 +                src/bin/memmgr/tests/Makefile
                   src/bin/msgq/Makefile
 }}}

 These two messages look like saying a very similar thing both. Is one a
 never version of the other?
 {{{
 $(man_MANS):
         @echo Man generation disabled.  Creating dummy $@.  Configure with
 --enable-generate-docs to enable it.
         @echo Man generation disabled.  Remove this file, configure with
 --enable-generate-docs, and rebuild BIND 10 > $@

 endif
 }}}

 This date seems outdated. What does it mean?
 {{{
   <refentryinfo>
     <date>June 11, 2012</date>
   </refentryinfo>
 }}}

 The word „sharable“ seems strange here. It sounds like something that can
 be shared, but commonly isn't. We probably want to say „shared“ (because
 it usually would be shared, cases when it is created and not read by auth
 would be rare and usually either temporary or configuration error).
 {{{
     <para>The <command>b10-memmgr</command> daemon manages sharable
       memory segments storing in-memory DNS zone data, and
       communicates with other BIND 10 modules about the segment
       information so the entire system will use these segments
       in a consistent manner.
 }}}

 Can this be removed completely, instead of commenting it out (in
 memmgr.py.in):
 {{{
 #from isc.log import DBGLVL_TRACE_BASIC
 }}}

 Will this load all the local memory segments to memory, or do I confuse
 this `use_cache` with some other?
 {{{#!python
 DataSrcClientsMgr(use_cache=True)
 }}}

 Can it happen this is not configured? Don't we have some default? I'm not
 saying we should not check for the error condition, but the description
 sounds like the user forgot to configure something, while it is probably
 much deeper problem.
 {{{
 % MEMMGR_NO_DATASRC_CONF failed to add data source configuration: %1
 The memmgr daemon tried to incorporate data source configuration
 on its startup but failed to do so.  The most likely cause of this
 is that data sources are not simply configured.  If so, they must be.
 The memmgr daemon cannot do any meaningful work without data sources,
 so it immediately terminates itself.
 }}}

 This bit of code looks somewhat strange. What is the reason for it?
 {{{#!python
         except Exception:
             raise
 }}}

 Should the `o` be there in `0o500`? Is it some strange python syntax for
 octal?
 {{{#!python
         os.mkdir(self.__test_mapped_file_dir, 0o500) # drop writable bit
 }}}

 This looks like twice the same code.
 {{{#!python
         # If data source isn't configured it's considered fatal.
         self.__mgr.mod_ccsession.add_remote_exception = \
             isc.config.ModuleCCSessionError('faked exception')
 self.assertRaises(isc.server_common.bind10_server.BIND10ServerFatal,
                           self.__mgr._setup_module)

         self.__mgr.mod_ccsession.add_remote_exception = \
             isc.config.ModuleSpecError('faked exception')
 self.assertRaises(isc.server_common.bind10_server.BIND10ServerFatal,
                           self.__mgr._setup_module)
 }}}

 What is the motivation to returning the JSON _string_ here? I know there
 are cases when we need to pass the string (usually to the C++ wrappers),
 but even from the description, we expect there'll be cases when we pass it
 as structures (when sending as the command parameters) and it is handled
 as structures in the tests. So, does it make sense to convert to string
 and then back?
 {{{
 It returns a json expression in string that contains parameters for
 the specified type of user to reset a zone table segment with
 }}}

 Is the xor really the easiest way to write it? I'm not pushing it, but I
 find `1 - __writer_ver` more readable version for me:
 {{{#!python
 self.__writer_ver ^= 1
 }}}

 I believe this is wrong approach, since the test will show up as „passed“
 on system without shared memory. We should use the decorator to skip the
 test (I think it is `@skip-if`), as it shows correctly the test is
 skipped.

 Does a general server receive updates?
 {{{#!python
 % PYSERVER_COMMON_SERVER_STARTED %1 server has started
 The server process has successfully started and is now ready to receive
 commands and updates.
 }}}

 In the `test_select_interrupted`, why doesn't the second call to select
 cause call to the `check_command`?

 Also, but you may already know, I don't really agree with the generations,
 because:
  * It seems fragile.
  * Depending on implementation, it shows an internal knob that should not
 be tweaked to user.
  * It will require nontrivial or hackish changes to the configuration
 system.

 I was thinking, do we need to be able to compare which configuration is
 older, or is it OK to know if the configuration is the same or not? Then
 we could use some hash of the whole configuration (or JSON representation)
 instead of increasing numbers. Or, do we need that at all? Can't we just
 apply the map command from memmgr to the configuration that is in auth at
 the moment, and re-apply it once the auth is reconfigured?

 I know it is not implemented in this branch, but do we want to have the
 `load_zone` command, or should we start using the notifications for zone
 changes? If we have the `load_zone` command, we still need to update all
 the DDNS and XfrIn modules, so we could just update it to send the
 notification instead of hardcoding yet another module name into them.

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