BIND 10 #2862: update b10-auth to recognize data source memory segments

[BIND 10 Development]

#2862: update b10-auth to recognize data source memory segments
-------------------------------------+-------------------------------------
            Reporter:  jinmei        |                        Owner:
                Type:  task          |  vorner
            Priority:  medium        |                       Status:
           Component:  b10-auth      |  reviewing
            Keywords:                |                    Milestone:
           Sensitive:  0             |  Sprint-20130806
         Sub-Project:  DNS           |                   Resolution:
Estimated Difficulty:  5             |                 CVSS Scoring:
         Total Hours:  0             |              Defect Severity:  N/A
                                     |  Feature Depending on Ticket:
                                     |  shared memory data source
                                     |          Add Hours to Ticket:  0
                                     |                    Internal?:  0
-------------------------------------+-------------------------------------
Changes (by muks):

 * owner:  muks => vorner


Comment:

 Hi Michal

 Replying to [comment:10 vorner]:
 > > We can also have multiple instances of auth where, for example, just
 one
 > > process fails to map it. In this case, there'd arise the case where
 some
 > > auth processes are serving an old segment and some are serving the
 new.
 >
 > Hmm. It seems there are monsters hidden :-|.
 >
 > I changed it to aborting in such cases, because I don't think I can
 > just stop using a segment for one, the reset, as you say, doesn't
 > guarantee much about exception safety and not answering anything is
 > bad as well. There seems to be no reasonable thing to do.

 I agree with this too. If we are unable to remap a mapped file, there
 may be bigger underlying problems and it's better to abort verbosely.

 > As I changed it to abort, the MemMgr will get unsubscription
 > notification from the auth server in question.

 We should document somewhere (in the design document) that auth aborts
 if it doesn't remap successfully (as an implicit notification to
 memmgr).

 > > * I couldn't figure this out quickly reading the auth code, but is
 > >   `doSegmentUpdate()` synchronized with the rest of auth? I.e., when
 the
 > >   builder thread is resetting the segment, does auth respond to
 queries
 > >   during that time?
 >
 > To best of my knowledge, it stops answering the queries during the
 > switch of segments. The `doSegmentUpdate` acquires the mutex. And the
 > holder object (containing a locked mutex) is also held for the whole
 > time of looking up an answer, in auth_srv.cc.

 If it stops answering queries gracefully, that's fine (i.e., it's not in
 the middle of using `InMemoryClient` or something similar).

 > > * I don't see `DATASOURCE_CONFIGURED` at all in the code. Is this
 > >   supposed to be handled in `reconfigureDone()` ? From the ticket
 > >   description does the first `DATASOURCE_CONFIGURED` map to
 > >   `listsReconfigured()` and the second to `reconfigureDone()` ? I'm
 > >   afraid I don't follow this at all in the ticket description:
 > >
 > > > Also, if this is the last outstanding segment, it sends the
 DATASRC_CONFIGURED command with {"cache-state": INUSE}.
 >
 > Well, #2861 was done slightly differently than suggested in the
 > ticket. The ticket suggested to have another command queue in the
 > other direction. Instead, I added a callback that is called after the
 > command is completed. The other seemed more natural and cleaner to me.
 >
 > So, the command `DATASOURCE_CONFIGURED` does not exist. I also got
 > confused about that part of the description, but in my understanding,
 > we need to look for two events ‒ when we get complete new
 > configuration (`listsReconfigured` handles this one) and to tell
 > MemMgr we're using the new segment (which is `reconfigureDone`). I
 > don't see any other place to handle stuff and I think that your
 > mapping of the proposed `DATASOURCE_CONFIGURED` is correct (or, it
 > matches my understanding of the purpose of the ticket).

 OK we'll revisit this if needed in the other tickets. :)

 It's worrying to me that I don't follow why everything is designed the
 way it is.

 I've pushed a minor message descriptions update to the branch. Please
 check if it is ok.

 You can go ahead and merge this.

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