BIND 10 #1068: support writability in the new data source API

BIND 10 Development do-not-reply at isc.org
Mon Aug 22 16:18:21 UTC 2011


#1068: support writability in the new data source API
-------------------------------------+-------------------------------------
                   Reporter:         |                 Owner:  jinmei
  jinmei                             |                Status:  reviewing
                       Type:  task   |             Milestone:
                   Priority:  major  |  Sprint-20110830
                  Component:  data   |            Resolution:
  source                             |             Sensitive:  0
                   Keywords:         |           Sub-Project:  DNS
            Defect Severity:  N/A    |  Estimated Difficulty:  7.0
Feature Depending on Ticket:         |           Total Hours:  0
        Add Hours to Ticket:  0      |
                  Internal?:  0      |
-------------------------------------+-------------------------------------
Changes (by vorner):

 * owner:  vorner => jinmei


Comment:

 Hello

 As I probably had changes from 1183 in the diff as well, I might comment
 on something from there. But AFAIK you reviewed that one, so if it is not
 your code, please ignore it.

 So, the comments:
  * It doesn't compile for me, because `ASSERT_EQ` returns and is used in
 the `SQLite3Update` constructor:
 {{{
 sqlite3_accessor_unittest.cc: In constructor
 ‘<unnamed>::SQLite3Update::SQLite3Update()’:
 sqlite3_accessor_unittest.cc:302:60: error: returning a value from a
 constructor
 }}}
    May I suggest either throwing or using EXPECT_EQ, so people will know
 the follow-up errors are probably related to the failure? Or maybe there's
 an official solution to this?
  * Comment of `TTL_COLUMN = 1` is incomplete (terminating at an `a`).
  * Comment of `ADD_TTL` ‒ it says it is integer. But the actual data type
 is string, this wording is probably confusing. Something like „string
 representation of integer in decimal representation“?
  * The order of columns is different in the read and write methods (eg.
 the name is at the end in reading, at the beginning when writing).
 Wouldn't it be cleaner if they had the same order (with obvious variations
 around columns present only at one method)? They can be freely reordered
 in the SQL command, so it shouldn't be a technical problem.
  * The documentation of getRecords uses part of description for
 getAllRecords, which does not apply completely (for example it talks about
 order of RRs from different RRsets).
  * To make the interface consistent, should the vectors be replaced by
 fixed-sized arrays and their sizes be checked compile-time by the
 reference trick?
  * There are enums used as indices and with the last _COUNT item to mark
 the count. On one side, we have a requirement to manually number enums in
 our style guide (I think I saw it somewhere), but maybe in this and
 similar cases, we should reconsider. The standard says an enum starts at 0
 and each next item is one larger than previous, if the number is free,
 which is exactly what we want in this case (numbering by one, starting at
 0). Numbering manually is error prone, since human might just add one and
 not update all the previous ones, or something. Should we discuss it on
 the ML?
  * Isn't the agent for execution called „executioner“ instead of
 „executer“ (sorry, I'm not native speaker, but I did not hear the second).
  * The `startUpdateZone` returns an ID, but the ID isn't used to anything
 (not a parameter to any of the updating methods). As you note, it might
 not correspond to the one from `getZone`, so it can't be used for reading
 as well. What is it's purpose? I guess current API doesn't allow updating
 multiple zones at once (even in case the underlying DB might).
  * Is it guaranteed that the application will see the updated data in the
 transaction, or is it implementation detail of the SQLite one? Is it
 allowed for some backend to create a separate connection for the updates
 and have any reads out of that transaction? Do we support DBs that don't
 support transactions (eg. text files, CVS files, some tables of MySQL)?

 With regards

-- 
Ticket URL: <http://bind10.isc.org/ticket/1068#comment:7>
BIND 10 Development <http://bind10.isc.org>
BIND 10 Development


More information about the bind10-tickets mailing list