BIND 10 #1596: Addressing of multiple instances of the same component

[BIND 10 Development]

#1596: Addressing of multiple instances of the same component
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  vorner                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20120221
                  Component:         |            Resolution:
  Unclassified                       |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  5
Feature Depending on Ticket:         |           Total Hours:  0
  Multiple auths                     |
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 Replying to [comment:6 jinmei]:
 > - When I tried to add an additional b10-auth process (from the source
 env):
 > [...]
 >   with the attached config (it uses non privileged port), two
 >   processes in fact ran, but only one of them seemed to listen on:

 That is to be expected. This is solved in #1595, which is already merged
 in master, but not in this branch yet. If you merge with master locally,
 you should see that it works (or you could trust me I tried it O:-).

 > - It'd be convenient if I could differentiate different instances of
 >   the same type of process in logs.

 Hmm, I guess it could be done somehow. But I think it would be a different
 ticket.

 > - I'm now feeling I'm confused.  What was the expected relationship
 >   between components and processes?  I thought a single component can
 >   consist of multiple (same type of) processes, but the updates to
 >   bind10 and component.py seem to assume it's one-to-one
 >   relationship.  Comments and documentation also seem to be not very
 >   consistent.  bind10_src.py says:
 > {{{#!python
 >         # Some time in future, it may happen that a single component has
 >         # multple processes. If so happens, name "components" may be
 >         # inapropriate. But as the code isn't probably completely ready
 >         # for it, we leave it at components for now.
 >         self.components = {}
 > }}}
 >   which seems to support the one-to-multi design.  On the other hand,
 >   component.py says:
 > {{{
 > This framework allows for a single process to be started multiple times
 (by
 > specifying multiple components with the same configuration). However,
 the rest
 > of the system might not handle such situation well, so until it is made
 so,
 > it would be better to start each process at most once.
 > }}}
 >   which seems to support the one-to-one mapping.

 Well, when I wrote the first comment, I thought about a component that is
 a pipeline, for example, so there would be two different processes that do
 the one thing. This component could still be started multiple times by
 putting it multiple times into the configuration.

 Anyway, I'd like to be able to avoid specifying the same component
 multiple times, I'd rather have a count = something configuration element
 in each of them. If the boss then generates multiple component objects or
 just one is question for the future implementation, though. And I'd like
 to try this multiple processes thing out first a little before investing
 too much time into fine-tuning the configuration of it.

 > - As for the testability of resolver, I see it's difficult with the
 >   current implementation architecture.  For a longer term, I
 >   personally think we should unify command/config submodule for auth
 >   and resolver and share as much code as possible.  But in any case
 >   it's beyond the scope of this task.

 ACK.

 > - A related note: since this feature involves multiple processes, I
 >   think we need system tests.  Then we can also test the resolver at
 >   least at the higher level.  It could be okay to defer it to a
 >   separate task, but I think it should be done pretty soon (e.g., in
 >   the next sprint).

 Hmm, I'd like to merge it to master first (just to make sure both of them
 listen on the port). It could be quite easy to do the tests then, like
 starting two copies, sending a query, stopping one of them, sending a
 query, starting it again, stopping the first one (to verify the sharing of
 sockets), send the query again, then shout them both down and see that a
 query times out. So, I'd create a new ticket and put it to next-splint-
 proposed.

 > '''auth_srv.h'''
 >
 > Just checking: why did you remove this?  Because it's trivial?
 > {{{
 > -    /// \return the value of the counter.
 > +
 > }}}

 I don't remember removing it, so I probably hit some random keys in vi
 when looking into the file.

 > '''auth/command.cc'''
 >
 > - Is there a rationale for the ordering between <string> and boost
 >   headers?  Also, if the basic rule is "if header A (possibly) depends
 >   on header B, include A first", dns/ headers should be included much
 >   later.  Same for exceptions.h.  Same for other files.

 I don't think there's a rationale behind this. I just reordered them to
 have local headers first, then the project headers and then system
 headers/library headers. I don't usually think about it to that much
 detail, just about the three groups (with the same-named header going
 first in the local group). I reordered them little bit by some kind of gut
 feeling, I think it should be enough (the other project headers would go
 first in their directory, so they'd be tested there).

 > - what if args is bogus (e.g. pid is not int, perhaps there can be
 >   other bogus cases)?  it should be tested.  same for resolver, and I
 >   suspect it can be more devastating because exceptions won't be
 >   caught at a lower level.

 Ah, you're right it could be something else than int. I check for that
 now, but I don't see any other bogus input (negative numbers would be
 handled implicitly as not equal, etc).

 > - maybe we should log the shutdown event?  Especially if the wrong pid
 >   is not expected to be delivered (I was not sure about that), that
 >   would be worth noting.

 It is expected to come, see (newly added) comment.

 On a slightly related note, I think we will need to do some redesign near
 the configuration, commands and communication and how they should work
 when there's more than one instance of a component. This way it is more
 like a hackish way to make it work than a proper way.

 > - ShutdownCommand::exec: I'd rather save a few lines by not
 >   introducing pid and my_pid.  again, it may be a matter of
 >   preference.

 I find this slightly more readable, so I'd like to keep it this way.

 > - AuthCommandTest::shutdownCorrectPID: another matter of taste, but I
 >   don't need the temporary 'param' if we make the member 'param'
 >   variable mutable (normal ElementPtr) and reset it.

 I don't know, I don't think that one needs to be mutable.

 > - do we need to regenerate and merge HTML in individual branch?

 Shouldn't I regenerate it when I change it? (it's always a problem when we
 have a generated file in the repository). Anyway, I wanted to the output,
 so I needed to regenerate it. Should I revert the html back?

 > - I suspect there's no SQLite3 locking issue due to using multiple
 >   auth instances (at least for now auth only uses it for read-only, so
 >   multiple auth instances shouldn't content each other).  But the
 >   guide says "could be", which is basically always correct by
 >   definition, I don't oppose to that if you really want to mention
 >   that possibility.

 I'd like to keep it there. There were several surprises from SQLite for us
 about logging already, so I expect the worst there.

 > '''changelog'''
 >
 > Maybe "core" is more commonly used than CPU today?  (even though
 > "multi-core" has also limited meaning).  Not a strong opinion anyway.

 OK, I'll use core when I merge it (and I hope I won't forget).

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