[bind10-dev] proposal: a performance extendable auth query logic framework

[JINMEI Tatuya / 神明達哉]

Thanks for the prompt feedback.

At Wed, 29 Feb 2012 10:11:21 +0100,
Michal 'vorner' Vaner <michal.vaner at nic.cz> wrote:

> > The main idea is to update the return type of ZoneFinder::find() to an
> > abstract class named 'FindContext'.  It can store various kinds of
> > internal information of the find() call, which are possibly specific
> > to the underlying data source (e.g. for the in-memory version, it
> > would contain the rbtnode that matched in the original search, etc).
> > 
> > FindContext provides a set of methods that handle several post-find
> > operations such as getting the additional RRset corresponding to the
> > original find() result (A/AAAA for NS, etc), providing DNSSEC proofs,
> > synthesizing CNAME with DNAME:
> 
> The idea itself looks good to me. However, I'm not sure how big overhead it will
> be to have another abstract class, allocated on the heap, in the main path.

Yeah I was aware of it.  My undocumented assumption is that we'd
eventually use some technique to minizmie this overhead.  For example,
I believe we can use a mixture of boost::object_pool and shared_ptr.
The underlying ZoneFinder object maintains the pool, and reuse the
pooled object whenever possible.  We set the shared_ptr's destructor
so that when the final reference to the context is released it will go
back to the pool, rather than be deleted.

> Also, is it OK to reload zone while the context still lives? (I know this would
> not happen in auth).

It probably wouldn't, but it shouldn't be different from whether a
ZoneFinder is valid when the corresponding zone is reloaded (it may or
not be so depending on the implementation detalis, but in general we
should probably assume it's not safe).  So I believe the introduction
of FindContext doesn't introduce a new problem on this point.

> Wouldn't it be better to append the produced RRsets directly to the Message? Or
> why are the intermediate vector used?

That's a good point, too, it's related to another possible issue of
this approach that I didn't mention in the previous message.  In some
sense the concept of FindContext violates the API layer separation in
that it would require the data source module be aware of too much
about higher level query handling - the clearer separation was one of
the main motivations of the newer API, but FindContext will break it
again.  It's a compromise to give some special data source
implementations the opportunity of fully optimizing the lookup
performance at a higher level, while still minimizing the requirement
of hihger layer knowledge for most of the other data source
implementations.  The default implementations of the FindContext
methods ensure that.

And, while already introucinng some layer violation, I still wanted to
keep the data source module as much independent from the high level
query logic as possible.  That's one reason why FindContext doesn't
take dns::Message.  This will also give a caller more flexibility of
the result of FindContext methods - although it may not be realistic,
an application could use the result for other purposes than
constructing a response message.

Another reason is that I expect the caller (Query::process()) will
have to do some post-lookup tasks before actually inserting the RRsets
to the response message.  One known thing is to suppress duplicate
RRsets.  At least for now there's no way to remove an RRset from a
message section, so the caller needs to do it before inserting them to
the message.

In terms of performance, it's a kind of waste that FindContext inserts
RRsets to the vector and the caller iterates over and copies them to
the message.  But considering the entire workload of query processing,
I believe/hope it's relatively minor.  If it turns out the resulting
total performance is still insufficient and this overhead is
significant, we can then consider optimizing it further, even if it
breaks the layering assumptions more.

---
JINMEI, Tatuya
Internet Systems Consortium, Inc.