BIND 10 #2862: update b10-auth to recognize data source memory segments
BIND 10 Development
do-not-reply at isc.org
Tue Jul 23 11:56:11 UTC 2013
#2862: update b10-auth to recognize data source memory segments
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner: muks
Type: task | Status:
Priority: medium | reviewing
Component: b10-auth | Milestone:
Keywords: | Sprint-20130723
Sensitive: 0 | Resolution:
Sub-Project: DNS | CVSS Scoring:
Estimated Difficulty: 5 | 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 => muks
Comment:
Hello
Replying to [comment:9 muks]:
> > There are some new functionalities in the `ModuleCCSession`. It seems
> > we are bending it for something that it doesn't want to do right now
> > (pretending to be some other module). Also, part of that new
> > functionality is not really clean. I needed something, but the ticket
> > is taking a long time already and it didn't seem directly relevant, so
> > I hacked it in. But a real solution would be needed.
>
> I am not very familiar with CC so I ask.. what is the proposed solution
> to replace the code that you have put in place?
Well, I don't have a very concrete idea. But:
* It should be possible to register multiple independent callbacks,
instead of just one.
* Maybe we should be able to register them as handler for given group
(eg. call only if the command is directed to group of given name, or if
registered for „direct“ commands ‒ ones sent by lname and not by group).
* Maybe register based on command name.
> > Also, there's a related part of the auth code that is not tested,
> > because it sounds it is very hard to test (see the last commit). I
> > think this will be better tested by a lettuce test (and that we'll
> > need one in the end anyway, because I'm bit worried if the
> > communication in auth will match the one in the memmgr).
>
> Nod. Please add a ticket towards this so it's not forgotten, and add it
> to the meta ticket to do after the memmgr implementation is in place.
OK, I'll add it on the merge of this ticket, as usual.
> > So, there are two possibilities. Create a ticket to clean up the
> > `ModuleCCSession`, check that there's a ticket for the lettuce
> > test. Or the other is to redesign the communication with primitives we
> > already have (eg. notifications and commands).
>
> See above. I'm not familiar with how CC is lacking here. Please explain
> what you propose.
It would be possible to turn the communication inside out in a sense.
Currently, it works by:
* Auth subscribes.
* MemMgr sends command „Use this segment!“. This is slightly problematic,
since the current CC classes don't expect commands to be send directly by
lname (the unique identifier on the bus), but by a module name/group.
* Auth sends command (hmm, bad name) „I use this segment“.
The other possibility is something:
* Auth subscribes.
* MemMgr publishes notification „This segment shall be used“.
* Whenever auth starts using a segment, it notifies others in the group
„I'm using this segment“.
The Auth will know what to use. And the memmgr can keep trac of what files
are used by which auth.
> 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.
> * The opposite of "local" is not "remote", but "mapped" in the case of
> `hasRemoteSegment()`. It should be renamed to
> `hasMappedSegment()`. Also I think it's better to specifically search
> for "mapped" from `getSegmentType()` instead of !"local".
Well, my thinking was, it is possible there will be different mechanisms
to synchronize the segments than just mapping of files. So I was thinking
that I'd handle all of these. But maybe we can be on the safe side.
> * Is it possible (or meaningful) to have a mapped segment and
> `cached-enable: false`?
Once the cache is disabled, all the other cache options are just ignored.
I don't think the code would reject config with this setup, but it would
just have no effect.
> * The "segment_info_update" command seems renamed to
> "sgmtinfo-update". Is there any reason for this, when most of the
> design documentation (in other tickets also) refers to
> "segment_info_update"? Same with "sgmtinfo-update-ack".
The reason was I took it from other document than you, probably. I'm sure
I did copy the really strange-looking name of the command from somewhere,
I didn't come up with this. Anyway, fixed.
> * In the "sgmtinfo-update-ack", are you sending back all the params sent
> with the "segment_info_update" command? Isn't it enough to send the
> class and data source name?
It would probably be enough, at least now. But:
* If I wanted to send only them, I'd have to construct the answer and it
would be needlessly more code.
* This way, the sender (MemMgr) can store any other tracking information
it may need to identify the ack inside (for example the version of config
it was applied to, or something), without touching the auth server again.
So it seems more generic this way.
> * If there is a failure, shall we add an arg to notify memmgr of a
> failure to reset condition in the "sgmtinfo-update-ack" reply (in any
> case, regardless of how auth decides to handle it internally - see
> discussion above)?
As I changed it to abort, the MemMgr will get unsubscription notification
from the auth server in question.
> * 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.
> * 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).
--
Ticket URL: <http://bind10.isc.org/ticket/2862#comment:10>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list