BIND 10 #249: review: auth server can return SERVFAIL with an answer

[BIND 10 Development]

#249: review: auth server can return SERVFAIL with an answer
--------------------------+-------------------------------------------------
 Reporter:  jinmei        |        Owner:  jinmei                                        
     Type:  defect        |       Status:  reviewing                                     
 Priority:  major         |    Milestone:  05. 3rd Incremental Release: Serious Secondary
Component:  Unclassified  |   Resolution:                                                
 Keywords:                |    Sensitive:  0                                             
--------------------------+-------------------------------------------------
Changes (by each):

  * owner:  UnAssigned => jinmei


Comment:

 Replying to [comment:1 jinmei]:
 > I decided to fundamentally refactor the test data source mock.

 Glorious.  Vast improvement.  Thank you.  Merge it.

 > Notes on the bug and fix themselves:

 Rather than throwing exceptions from the data source, I would have had
 b10-auth look for a SERVFAIL Rcode in the reply from doQuery(), and either
 clear the partial answers out of the Message or construct a new Message
 with no answer.  But, if we're going to throw exceptions, then I agree
 with your speculation that we should do so in every SERVFAIL case, not
 just the two that you've already done.

 > This observation leads to broader discussions.  First, I suspect other
 internal SERVFAIL cases should actully be changed to exceptions.  But I
 was not so sure if they are due to something like a broken database or an
 unsual, but not necessarily broken error.  So I didn't touch them.  We
 should revisit these cases, too, and, whatever the conclusion is, we
 should avoid SERVFAIL+answer for these cases.

 AFAIK, SERVFAIL always means either that the database is insane, or we've
 been told to do something unnatural (for example, a glue query found on
 the task queue, which should never happen).  Both seem like natural
 circumstances for an exception to me.

 My only question about it is, I recall having a conversation early in the
 BIND 10 project about someone (maybe you?) running benchmarks that showed
 exceptions caused a serious performance hit.  If that's true, then maybe
 it would be preferable to continue signaling the problem via the Rcode,
 and simply handle it better on the calling end?  But I'm okay either way.

 > So, in a longer term, I think we need to define a border in the top
 level data source, under which we cannot assume the validity of the data
 and the module must perform validation.

 What do you mean by "the module"?

-- 
Ticket URL: <https://bind10.isc.org/ticket/249#comment:3>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development