BIND 10 #1455: Add prerequisite checks to the DDNS module

[BIND 10 Development]

#1455: Add prerequisite checks to the DDNS module
-------------------------------------+-------------------------------------
                   Reporter:  jelte  |                 Owner:  jelte
                       Type:  task   |                Status:  reviewing
                   Priority:         |             Milestone:
  medium                             |  Sprint-20120529
                  Component:  DDNS   |            Resolution:
                   Keywords:         |             Sensitive:  0
            Defect Severity:  N/A    |           Sub-Project:  DNS
Feature Depending on Ticket:  DDNS   |  Estimated Difficulty:  6
        Add Hours to Ticket:  0      |           Total Hours:  0
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jelte


Comment:

 Hello

 Replying to [comment:13 jelte]:
 > > As I mentioned in the review of #1512, I consider the thing with
 delayed conversion (`RRsetFormatter` in this case) to string as a
 premature optimisation. This is python and concatenating two strings is
 pretty cheap operation, relative to other things. I fear that the
 allocation of the temporary object that will or will not be converted to
 the string and its initialization may be if not more expansive than the
 conversion, than at least comparably expensive. Therefore I don't really
 like the creation of classes whose only point of existence is to delay
 something. I suggested to Jinmei that a comment saying that it is meant
 for optimization, but we don't really know if it helps would be enough,
 but I don't want more of these things to be appearing in our code.
 > >
 >
 > A better way to put it would be 'string concatenation is expensive, but
 so is everything' :) (especially when doing multiple concats which is not
 the case here, but I digress). Good point. The formatters themselves (even
 if they were static functions that immediately performed the operation)
 are nice though, to get uniform output. We should probably move them to
 util though.
 >
 > For the sake of consistency I'll take over whatever 1512 ends up doing
 :)

 It did put a comment there. I think it does not need anything more and I
 don't think I need to review the comment again.

 Do you want to create a ticket for the move to utils (and maybe clean them
 up)?

 > > I also worry little bit about performance of using `find` or
 `find_all` to check if a name exists. It has some pretty complex handling
 and the `_all` one copies (and wraps to python) potentially a lot of
 RRsets.
 > >
 >
 > Are you suggesting we should have better find methods? (for instance
 similar ones to these, but only signal back that they *would* return
 something if the real version was called?)

 Well, I don't know if there would be any actual bottleneck, so not for
 now. But
 we may want to add a flag `FIND_I_DONT_CARE_ABOUT_FANCY_STUFF` and the
 find
 methods could just return an answer without doing all the processing.
 Maybe a
 comment in the code about the possible performance problem would be nice?

 > > Also, in the search if actual Rdata match, you remove Rdata from the
 RRSet. Is it safe? Can't it be the same RRset that is being preserved
 inside the In-Memory data source? In such case, the removals would
 actually destroy the RRset effectively. Would it be possible to just sort
 the Rdata by their textual representation (if they are not comparable
 directly)?
 > >
 >
 > Hmm. Given that you were just worried about performance, doing to-text
 and sorting would probably not be very efficient either (then again,
 neither is this current algorithm). I don't *think* we can actually delete
 data this way, but a relatively cheap solution would be to make a shallow
 copy of the list we are deleting from (so it'll only mess with refcounts).

 Well, the sorting would be `n·log n` + linear comparison, while your way
 it is `n²`. So, at least in theory for larger RRsets, the solution with
 sorting might be cheaper. But I guess the RRsets are generally small
 anyway, so let's not care about the performance here.

 > (as a proposal i committed the shallow copy as the final commit, we can
 choose not to use that and go for something else)

 That looks OK.

 I think only the comments are enough now (if you agree), so you can merge
 after putting them there.

 Thank you

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