BIND 10 trac324new2, updated. 698534e5564fb14a384d1cf28eda058453dcb1a5 [324] use a much larger major version (1000) for "newschema" test DBs.

BIND 10 source code commits bind10-changes at lists.isc.org
Mon Apr 2 18:20:02 UTC 2012


The branch, trac324new2 has been updated
       via  698534e5564fb14a384d1cf28eda058453dcb1a5 (commit)
       via  947f08c755bd91255b2f8034a65b99445f13b024 (commit)
       via  4df83ec7d1b92d7fb8c6fd2c0906694683da8c30 (commit)
      from  16ad60fe0d9ba8cf53fc6a190ad25d92ece8e8bd (commit)

Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.

- Log -----------------------------------------------------------------
commit 698534e5564fb14a384d1cf28eda058453dcb1a5
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Apr 2 11:17:54 2012 -0700

    [324] use a much larger major version (1000) for "newschema" test DBs.
    
    it will reduce the risk of getting conflict soon due to schema updates.

commit 947f08c755bd91255b2f8034a65b99445f13b024
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Apr 2 11:13:21 2012 -0700

    [324new2] renamed major/minor version variables so they are more consistent.

commit 4df83ec7d1b92d7fb8c6fd2c0906694683da8c30
Author: JINMEI Tatuya <jinmei at isc.org>
Date:   Mon Apr 2 11:07:50 2012 -0700

    [324] make sure initial SQLite3 DB creation is exception safe.
    
    this is not really an issue of this task, and I believe we should soon stop
    creating the schema in the accessor context in the first place, but the bug
    is real so I fixed it anyway.  Assuming this creation code and the older
    version of implementation will be gone soon, I simply duplicated the mostly
    same code in both old and new implementations.
    
    Also, it seemed extremely difficult (I'd say it's effectively impossible) to
    test these cases.  I couldn't come up with any reasonable way to test them,
    so I didn't add any test.

-----------------------------------------------------------------------

Summary of changes:
 src/lib/datasrc/sqlite3_accessor.cc                |   77 ++++++++++++++------
 src/lib/datasrc/sqlite3_datasrc.cc                 |   72 +++++++++++++------
 src/lib/datasrc/tests/testdata/newschema.sqlite3   |  Bin 2048 -> 2048 bytes
 .../isc/datasrc/tests/testdata/newschema.sqlite3   |  Bin 2048 -> 2048 bytes
 4 files changed, 104 insertions(+), 45 deletions(-)

-----------------------------------------------------------------------
diff --git a/src/lib/datasrc/sqlite3_accessor.cc b/src/lib/datasrc/sqlite3_accessor.cc
index 4c89760..9d6ba59 100644
--- a/src/lib/datasrc/sqlite3_accessor.cc
+++ b/src/lib/datasrc/sqlite3_accessor.cc
@@ -141,8 +141,8 @@ const char* const text_statements[NUM_STATEMENTS] = {
 
 struct SQLite3Parameters {
     SQLite3Parameters() :
-        db_(NULL), version_(-1), minor_(-1), in_transaction(false),
-        updating_zone(false), updated_zone_id(-1)
+        db_(NULL), major_version_(-1), minor_version_(-1),
+        in_transaction(false), updating_zone(false), updated_zone_id(-1)
     {
         for (int i = 0; i < NUM_STATEMENTS; ++i) {
             statements_[i] = NULL;
@@ -180,8 +180,8 @@ struct SQLite3Parameters {
     }
 
     sqlite3* db_;
-    int version_;
-    int minor_;
+    int major_version_;
+    int minor_version_;
     bool in_transaction; // whether or not a transaction has been started
     bool updating_zone;          // whether or not updating the zone
     int updated_zone_id;        // valid only when in_transaction is true
@@ -378,36 +378,67 @@ pair<int, int> checkSchemaVersion(sqlite3* db) {
     }
 }
 
+// A helper class used in createDatabase() below so we manage the one shot
+// transaction safely.
+class ScopedTransaction {
+public:
+    ScopedTransaction(sqlite3* db) : db_(NULL) {
+        // try for 5 secs (50*0.1)
+        for (size_t i = 0; i < 50; ++i) {
+            const int rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION",
+                                        NULL, NULL, NULL);
+            if (rc == SQLITE_OK) {
+                break;
+            } else if (rc != SQLITE_BUSY || i == 50) {
+                isc_throw(SQLite3Error, "Unable to acquire exclusive lock "
+                          "for database creation: " << sqlite3_errmsg(db));
+            }
+            doSleep();
+        }
+        // Hold the DB pointer once we have successfully acquired the lock.
+        db_ = db;
+    }
+    ~ScopedTransaction() {
+        if (db_ != NULL) {
+            // Note: even rollback could fail in theory, but in that case
+            // we cannot do much for safe recovery anyway.  We could at least
+            // log the event, but for now don't even bother to do that, with
+            // the expectation that we'll soon stop creating the schema in this
+            // module.
+            sqlite3_exec(db_, "ROLLBACK", NULL, NULL, NULL);
+        }
+    }
+    void commit() {
+        if (sqlite3_exec(db_, "COMMIT TRANSACTION", NULL, NULL, NULL) !=
+            SQLITE_OK) {
+            isc_throw(SQLite3Error, "Unable to commit newly created database "
+                      "schema: " << sqlite3_errmsg(db_));
+        }
+        db_ = NULL;
+    }
+
+private:
+    sqlite3* db_;
+};
+
 // return db version
 pair<int, int>
 createDatabase(sqlite3* db) {
+    logger.info(DATASRC_SQLITE_SETUP);
+
     // try to get an exclusive lock. Once that is obtained, do the version
     // check *again*, just in case this process was racing another
-    //
-    // try for 5 secs (50*0.1)
-    int rc;
-    logger.info(DATASRC_SQLITE_SETUP);
-    for (size_t i = 0; i < 50; ++i) {
-        rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION", NULL, NULL,
-                            NULL);
-        if (rc == SQLITE_OK) {
-            break;
-        } else if (rc != SQLITE_BUSY || i == 50) {
-            isc_throw(SQLite3Error, "Unable to acquire exclusive lock "
-                        "for database creation: " << sqlite3_errmsg(db));
-        }
-        doSleep();
-    }
+    ScopedTransaction trasaction(db);
     pair<int, int> schema_version = checkSchemaVersion(db);
     if (schema_version.first == -1) {
         for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
             if (sqlite3_exec(db, SCHEMA_LIST[i], NULL, NULL, NULL) !=
                 SQLITE_OK) {
                 isc_throw(SQLite3Error,
-                        "Failed to set up schema " << SCHEMA_LIST[i]);
+                          "Failed to set up schema " << SCHEMA_LIST[i]);
             }
         }
-        sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
+        trasaction.commit();
 
         // Return the version.  We query again to ensure that the only point
         // in which the current schema version is defined is in the create
@@ -438,8 +469,8 @@ checkAndSetupSchema(Initializer* initializer) {
             .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
     }
 
-    initializer->params_.version_ = schema_version.first;
-    initializer->params_.minor_ = schema_version.second;
+    initializer->params_.major_version_ = schema_version.first;
+    initializer->params_.minor_version_ = schema_version.second;
 }
 
 }
diff --git a/src/lib/datasrc/sqlite3_datasrc.cc b/src/lib/datasrc/sqlite3_datasrc.cc
index 8a91987..b450cd5 100644
--- a/src/lib/datasrc/sqlite3_datasrc.cc
+++ b/src/lib/datasrc/sqlite3_datasrc.cc
@@ -50,14 +50,14 @@ namespace isc {
 namespace datasrc {
 
 struct Sqlite3Parameters {
-    Sqlite3Parameters() :  db_(NULL), version_(-1), minor_(-1),
+    Sqlite3Parameters() :  db_(NULL), major_version_(-1), minor_version_(-1),
         q_zone_(NULL), q_record_(NULL), q_addrs_(NULL), q_referral_(NULL),
         q_any_(NULL), q_count_(NULL), q_previous_(NULL), q_nsec3_(NULL),
         q_prevnsec3_(NULL)
     {}
     sqlite3* db_;
-    int version_;
-    int minor_;
+    int major_version_;
+    int minor_version_;
     sqlite3_stmt* q_zone_;
     sqlite3_stmt* q_record_;
     sqlite3_stmt* q_addrs_;
@@ -743,28 +743,56 @@ pair<int, int> check_schema_version(sqlite3* db) {
     }
 }
 
+// A helper class used in create_database() below so we manage the one shot
+// transaction safely.
+class ScopedTransaction {
+public:
+    ScopedTransaction(sqlite3* db) : db_(NULL) {
+        // try for 5 secs (50*0.1)
+        for (size_t i = 0; i < 50; ++i) {
+            const int rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION",
+                                        NULL, NULL, NULL);
+            if (rc == SQLITE_OK) {
+                break;
+            } else if (rc != SQLITE_BUSY || i == 50) {
+                isc_throw(Sqlite3Error, "Unable to acquire exclusive lock "
+                          "for database creation: " << sqlite3_errmsg(db));
+            }
+            do_sleep();
+        }
+        // Hold the DB pointer once we have successfully acquired the lock.
+        db_ = db;
+    }
+    ~ScopedTransaction() {
+        if (db_ != NULL) {
+            // Note: even rollback could fail in theory, but in that case
+            // we cannot do much for safe recovery anyway.  We could at least
+            // log the event, but for now don't even bother to do that, with
+            // the expectation that we'll soon stop creating the schema in this
+            // module.
+            sqlite3_exec(db_, "ROLLBACK", NULL, NULL, NULL);
+        }
+    }
+    void commit() {
+        if (sqlite3_exec(db_, "COMMIT TRANSACTION", NULL, NULL, NULL) !=
+            SQLITE_OK) {
+            isc_throw(Sqlite3Error, "Unable to commit newly created database "
+                      "schema: " << sqlite3_errmsg(db_));
+        }
+        db_ = NULL;
+    }
 
+private:
+    sqlite3* db_;
+};
 
 // return db version
 pair<int, int> create_database(sqlite3* db) {
-    // try to get an exclusive lock. Once that is obtained, do the version
-    // check *again*, just in case this process was racing another
-    //
-    // try for 5 secs (50*0.1)
-    int rc;
     logger.info(DATASRC_SQLITE_SETUP);
-    for (size_t i = 0; i < 50; ++i) {
-        rc = sqlite3_exec(db, "BEGIN EXCLUSIVE TRANSACTION", NULL, NULL,
-                            NULL);
-        if (rc == SQLITE_OK) {
-            break;
-        } else if (rc != SQLITE_BUSY || i == 50) {
-            isc_throw(Sqlite3Error, "Unable to acquire exclusive lock "
-                        "for database creation: " << sqlite3_errmsg(db));
-        }
-        do_sleep();
-    }
 
+    // try to get an exclusive lock. Once that is obtained, do the version
+    // check *again*, just in case this process was racing another
+    ScopedTransaction transaction(db);
     pair<int, int> schema_version = check_schema_version(db);
     if (schema_version.first == -1) {
         for (int i = 0; SCHEMA_LIST[i] != NULL; ++i) {
@@ -774,7 +802,7 @@ pair<int, int> create_database(sqlite3* db) {
                         "Failed to set up schema " << SCHEMA_LIST[i]);
             }
         }
-        sqlite3_exec(db, "COMMIT TRANSACTION", NULL, NULL, NULL);
+        transaction.commit();
 
         // Return the version. We query again to ensure that the only point
         // in which the current schema version is defined is in the
@@ -806,8 +834,8 @@ checkAndSetupSchema(Sqlite3Initializer* initializer) {
             .arg(SQLITE_SCHEMA_MAJOR_VERSION).arg(SQLITE_SCHEMA_MINOR_VERSION);
     }
 
-    initializer->params_.version_ = schema_version.first;
-    initializer->params_.minor_ = schema_version.second;
+    initializer->params_.major_version_ = schema_version.first;
+    initializer->params_.minor_version_ = schema_version.second;
     initializer->params_.q_zone_ = prepare(db, q_zone_str);
     initializer->params_.q_record_ = prepare(db, q_record_str);
     initializer->params_.q_addrs_ = prepare(db, q_addrs_str);
diff --git a/src/lib/datasrc/tests/testdata/newschema.sqlite3 b/src/lib/datasrc/tests/testdata/newschema.sqlite3
index 8f020cb..460cfa8 100644
Binary files a/src/lib/datasrc/tests/testdata/newschema.sqlite3 and b/src/lib/datasrc/tests/testdata/newschema.sqlite3 differ
diff --git a/src/lib/python/isc/datasrc/tests/testdata/newschema.sqlite3 b/src/lib/python/isc/datasrc/tests/testdata/newschema.sqlite3
index 8f020cb..460cfa8 100644
Binary files a/src/lib/python/isc/datasrc/tests/testdata/newschema.sqlite3 and b/src/lib/python/isc/datasrc/tests/testdata/newschema.sqlite3 differ



More information about the bind10-changes mailing list