BIND 10 #2854: memory manager framework
BIND 10 Development
do-not-reply at isc.org
Wed Jun 26 00:09:11 UTC 2013
#2854: memory manager framework
-------------------------------------+-------------------------------------
Reporter: jinmei | Owner:
Type: task | jinmei
Priority: medium | Status:
Component: shmem | reviewing
manager | Milestone:
Keywords: | Sprint-20130709
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
-------------------------------------+-------------------------------------
Comment (by jinmei):
Replying to [comment:12 vorner]:
> > Thanks for the review. I've addressed specific code comments, leaving
> > some higher level discussions (generation ID and load command) open.
> > I'll be offline for a while, so I'm giving the ticket back to you at
> > this point.
>
> OK. What do we do with the points? Defer to other tickets/mailing list?
The generation ID thing will probably need a ML discussion. It also
has implications for this branch to complete, so I'll make some
comments on that below.
> > > 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.
>
> And this one? Is it one of the higher level ones?
Ah, sorry, that was an oversight. On thinking about it, my first
impression is that it would rather be a noise that is not really
useful in practice. But I then realized there was a precedence for a
similar case: the DHCP-DDNS (so called "d2") daemon. Following that
convention, this is my proposed entry:
{{{
631.? [func] jinmei
b10-memmgr: a new BIND 10 module that manages shared memory
segments for DNS zone data. At this point it's runnable but does
nothing really meaningful for end users; it was added to the
master branch for further development.
(Trac #2854, git TBD)
}}}
> > > These two messages look like saying a very similar thing both. Is
one a never version of the other?
> > I see the point, but it seems to be derived from existing Makefile
> > (probably for ddns). And I'm not sure how it should be fixed - I
> > believe it was originally written by Jeremy. So my suggestion is to
> > create a separate cleanup ticket for all these rules and give it to
> > him.
>
> OK, let's do a separate ticket.
Created a ticket: #3012.
> > > 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)
> > > }}}
> >
> > Hmm, I didn't think about the implication, but I believe (a copy of)
> > local segments will be loaded for memmgr. And I guess you meant it
> > might be a waste and should be avoided - if so, I see the point, but
> > I think it can be at least deferred in practice; if the amount of
> > memory (and loading overhead itself) is substantial (and as long as
> > memmgr is used), it would better be shared anyway. If it's not that
> > big, that's a waste but marginal. So I suggest just creating a
> > ticket, describing the issue, and see if we want to solve it later.
>
> Yes, that was my worry. I'm OK with separate ticket.
Created a ticket: #3013.
> > > 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
> > > }}}
> >
> > I guess (again I forgot what exactly I was thinking) I thought we want
> > a string form so we can pass it to
> > `ConfigurableClientList.reset_memory_segment()`.
>
> That makes some sense (to pass it there as string). But I still believe
it is better to transform it to string there, than transform it from
string at 3 or 4 other places _and_ do the transform to string and back
from in these cases.
I concurred. Changed the interface as suggested.
> > > 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.
> > > }}}
> >
> > Not in the bind10_server module, but in the user class of that mixin.
> > I think the log description is okay as the observable behavior for end
> > users should be the same.
>
> Well, being a user, I would be wondering what kind of updates can
XfrOut, for example, receive. As a developer, I know these are updates of
configuration, but this might be confusing for user. This is what I mean.
Hmm, I'm afraid I still don't understand what you tried to raise, but
would it help if it reads "now ready to receive commands and
configuration updates."?
Finally, on higher level points. First, on the implication to the
current branch: understanding the concept of generation IDs will need
more discussion even if it might be eventually adopted, would you like
references to it to be removed from the branch? For example, this
means we'll update `DataSrcClientsMgr` so get_clients_map() only
returns the clients map. Or are we going to keep it for now and
update it later as we figure out how to deal with the issue?
The rest of the comments are basically outside the scope of this
ticket and would rather be a subject of project wide discussions, but
I'm making them for the record.
Replying to [comment:9 vorner]:
> 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 wouldn't necessarily defend the idea of configuration generations;
it's just one possible way to achieve the goal to ensure consistency
between multiple modules in terms of the versions of data source
configuration. If there is a better way to achieve that goal I'm okay
with that.
Specifically how to achieve it is definitely beyond the scope of this
ticket, and should probably be discussed at project wide. Since it's
quite unlikely we have the discussion and reach consensus within this
month, I'd basically leave the topic to the staying team members.
Your comments on the idea of generations don't really seem to be
compelling to me, though:
- The term "fragile" is vague and I'm not sure exactly what it means,
but to me the concept of generation IDs is a rather robust approach
to solve the issue by centralizing the versioning and explicitly
sharing the IDs by the other modules.
- The point about tweakable knob is indeed an implementation detail,
and IMO a minor one. We even already have such a thing: the global
configuration "version". As long as we keep such knobs, we need to
handle them as special cases to prevent accidental misuse. I
suspect the protection for the "version" is currently not really
sufficient to prevent intentional abuse, but the fact that we've not
seen any troubles due to that seems to suggest it would probably be
sufficient as long as we hide the knob from the front end user
interfaces such as bindctl. On the other hand, if we think we
should even make it fail safe for intentional abuse, (I believe)
we'll need to implement additional layer of defense, and that should
be generic, not specific to the "version" knob. And, then, we'll
address this concern in a generic manner.
- I admit it will require a non trivial extension. So we'll need some
design considerations and then implement it. But I expect we'll
need some generic framework for ensuring consistency of shared
configuration anyway, and I see the concept of generation ID is a
generic tool for that, not just an ad hoc extension to address the
specific issue of data source configuration.
Considering the third point, one major question is whether we can
address this specific issue with in a more trivial way. I don't have
a clear answer to that, and that will be the subject of followup
discussion, but I suspect we cannot simply rely on the "trivial"
approaches below:
> 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 suspect we'll need to distinguish the ordering of the configuration.
If the data source configuration changes from "C1", "C2", then "C1"
again, the first and third one will have the same hash (if we can
define the hash reasonably). A user module such as auth may look at
the first "C1", while the memmgr may have just restarted (and missed
two intermediate configuration updates) and look at the second "C1'.
In this case the user module will still wait for segments for "C2",
but memmgr can't answer that.
I'm not really sure what "just apply the map" means, but it means
something like auth is looking at data source config C1 and memmgr
gives memory segment information created from config C2, I don't think
we can rely on that. C1 and C2 may define the same name of data
source (probably by hand) with totally different underlying data.
> 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.
I'd leave it to you and the rest of the team. There's no implication
on this particular task, and I believe you have more specific ideas of
how we move forward about this.
--
Ticket URL: <http://bind10.isc.org/ticket/2854#comment:13>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development
More information about the bind10-tickets
mailing list