gpg: Provide an interface to patch TOFU updates.
authorNeal H. Walfield <neal@g10code.com>
Fri, 23 Oct 2015 15:23:17 +0000 (17:23 +0200)
committerNeal H. Walfield <neal@g10code.com>
Fri, 23 Oct 2015 15:38:17 +0000 (17:38 +0200)
* g10/tofu.c (struct db): Rename begin_transaction to savepoint_batch.
Rename end_transaction to savepoint_batch_commit.  Update users.
Remove field rollback.  Add fields savepoint_inner and
savepoint_inner_commit.  Add field batch_update.
(dump_cache): New function.
(batch_update): New variable.
(begin_transaction). New function.
(end_transaction): New function.
(rollback_transaction): New function.
(tofu_begin_batch_update): New function.
(tofu_end_batch_update): New function.
(closedb): End any pending batch transaction.
(closedbs): Assert that none of the DBs have a started batch
transaction if we not in batch mode.
(record_binding): Use the begin_transaction, end_transaction and
rollback_transaction functions instead of including the SQL inline.
Also start a batch mode transaction if we are using the flat format.
(tofu_register): Use the begin_transaction, end_transaction and
rollback_transaction functions instead of including the SQL inline.
* g10/gpgv.c (tofu_begin_batch_update): New function.
(tofu_end_batch_update): New function.
* g10/test-stubs.c (tofu_begin_batch_update): New function.
(tofu_end_batch_update): New function.

--
Signed-off-by: Neal H. Walfield <neal@g10code.com>
g10/gpgv.c
g10/keylist.c
g10/test-stubs.c
g10/tofu.c
g10/tofu.h

index 1d7dc74..ec09706 100644 (file)
@@ -632,3 +632,13 @@ tofu_policy_str (enum tofu_policy policy)
 
   return "unknown";
 }
+
+void
+tofu_begin_batch_update (void)
+{
+}
+
+void
+tofu_end_batch_update (void)
+{
+}
index 2a766a1..d4e6b74 100644 (file)
@@ -132,12 +132,16 @@ public_key_list (ctrl_t ctrl, strlist_t list, int locate_mode)
      which is associated with the inode of a deleted file.  */
   check_trustdb_stale ();
 
+  tofu_begin_batch_update ();
+
   if (locate_mode)
     locate_one (ctrl, list);
   else if (!list)
     list_all (ctrl, 0, opt.with_secret);
   else
     list_one (ctrl, list, 0, opt.with_secret);
+
+  tofu_end_batch_update ();
 }
 
 
index 4edea69..dfe6edb 100644 (file)
@@ -446,3 +446,13 @@ tofu_policy_str (enum tofu_policy policy)
 
   return "unknown";
 }
+
+void
+tofu_begin_batch_update (void)
+{
+}
+
+void
+tofu_end_batch_update (void)
+{
+}
index 3b8ced0..ad61536 100644 (file)
@@ -86,9 +86,11 @@ struct db
 
   struct
   {
-    sqlite3_stmt *begin_transaction;
-    sqlite3_stmt *end_transaction;
-    sqlite3_stmt *rollback;
+    sqlite3_stmt *savepoint_batch;
+    sqlite3_stmt *savepoint_batch_commit;
+
+    sqlite3_stmt *savepoint_inner;
+    sqlite3_stmt *savepoint_inner_commit;
 
     sqlite3_stmt *record_binding_get_old_policy;
     sqlite3_stmt *record_binding_update;
@@ -101,17 +103,35 @@ struct db
     sqlite3_stmt *register_insert;
   } s;
 
-
 #if DEBUG_TOFU_CACHE
   int hits;
 #endif
 
+  int batch_update;
+
   /* If TYPE is DB_COMBINED, this is "".  Otherwise, it is either the
      fingerprint (type == DB_KEY) or the normalized email address
      (type == DB_EMAIL).  */
   char name[1];
 };
 
+static struct db *db_cache;
+static int db_cache_count;
+#define DB_CACHE_ENTRIES 16
+
+static void tofu_cache_dump (struct db *db) GPGRT_ATTR_USED;
+
+static void
+tofu_cache_dump (struct db *db)
+{
+  log_info ("Connection %p:\n", db);
+  for (; db; db = db->next)
+    log_info ("  %s: %sbatch mode\n", db->name, db->batch_update ? "" : "NOT ");
+  log_info ("Cache:\n");
+  for (db = db_cache; db; db = db->next)
+    log_info ("  %s: %sbatch mode\n", db->name, db->batch_update ? "" : "NOT ");
+}
+
 #define STRINGIFY(s) STRINGIFY2(s)
 #define STRINGIFY2(s) #s
 
@@ -400,7 +420,160 @@ sqlite3_stepx (sqlite3 *db,
 
   return rc;
 }
+\f
+static int batch_update;
+
+/* Start a transaction on DB.  */
+static gpg_error_t
+begin_transaction (struct db *db, int only_batch)
+{
+  int rc;
+  char *err = NULL;
+
+  /* XXX: In split mode, this can end in deadlock.
+
+     Consider: we have two gpg processes running simultaneously and
+     they each want to lock DB A and B, but in different orders.  This
+     will be automatically resolved by causing one of them to return
+     EBUSY and aborting.
+
+     A more intelligent approach would be to commit and retake the
+     batch transaction.  This requires a list of all DBs that are
+     currently in batch mode.  */
+
+  if (batch_update && ! db->batch_update)
+    {
+      rc = sqlite3_stepx (db->db, &db->s.savepoint_batch,
+                          NULL, NULL, &err,
+                          "savepoint batch;", SQLITE_ARG_END);
+      if (rc)
+        {
+          log_error
+            (_("error beginning %s transaction on TOFU database '%s': %s\n"),
+             "batch", *db->name ? db->name : "combined", err);
+          sqlite3_free (err);
+          return gpg_error (GPG_ERR_GENERAL);
+        }
+
+      db->batch_update = 1;
+    }
+
+  if (only_batch)
+    return 0;
+
+  rc = sqlite3_stepx (db->db, &db->s.savepoint_inner,
+                      NULL, NULL, &err,
+                      "savepoint inner;", SQLITE_ARG_END);
+  if (rc)
+    {
+      log_error
+        (_("error beginning %s transaction on TOFU database '%s': %s\n"),
+         "inner", *db->name ? db->name : "combined", err);
+      sqlite3_free (err);
+      return gpg_error (GPG_ERR_GENERAL);
+    }
+
+  return 0;
+}
+
+/* Commit a transaction.  If ONLY_BATCH is 1, then this only ends the
+   batch transaction if we have left batch mode.  If ONLY_BATCH is 2,
+   this ends any open batch transaction even if we are still in batch
+   mode.  */
+static gpg_error_t
+end_transaction (struct db *db, int only_batch)
+{
+  int rc;
+  char *err = NULL;
+
+  if ((! batch_update || only_batch == 2) && db->batch_update)
+    /* The batch transaction is still in open, but we left batch
+       mode.  */
+    {
+      db->batch_update = 0;
+
+      rc = sqlite3_stepx (db->db, &db->s.savepoint_batch_commit,
+                          NULL, NULL, &err,
+                          "release batch;", SQLITE_ARG_END);
+      if (rc)
+        {
+          log_error
+            (_("error committing %s transaction on TOFU database '%s': %s\n"),
+             "batch", *db->name ? db->name : "combined", err);
+          sqlite3_free (err);
+          return gpg_error (GPG_ERR_GENERAL);
+        }
+
+      /* Releasing an outer transaction releases an open inner
+         transactions.  We're done.  */
+      return 0;
+    }
+
+  if (only_batch)
+    return 0;
+
+  rc = sqlite3_stepx (db->db, &db->s.savepoint_inner_commit,
+                      NULL, NULL, &err,
+                      "release inner;", SQLITE_ARG_END);
+  if (rc)
+    {
+      log_error
+        (_("error committing %s transaction on TOFU database '%s': %s\n"),
+         "inner", *db->name ? db->name : "combined", err);
+      sqlite3_free (err);
+      return gpg_error (GPG_ERR_GENERAL);
+    }
+
+  return 0;
+}
 
+static gpg_error_t
+rollback_transaction (struct db *db)
+{
+  int rc;
+  char *err = NULL;
+
+  if (db->batch_update)
+    /* Just undo the most recent update; don't revert any progress
+       made by the batch transaction.  */
+    rc = sqlite3_exec (db->db, "rollback to inner;", NULL, NULL, &err);
+  else
+    /* Rollback the whole she-bang.  */
+    rc = sqlite3_exec (db->db, "rollback;", NULL, NULL, &err);
+
+  if (rc)
+    {
+      log_error
+        (_("error rolling back inner transaction on TOFU database '%s': %s\n"),
+         *db->name ? db->name : "combined", err);
+      sqlite3_free (err);
+      return gpg_error (GPG_ERR_GENERAL);
+    }
+
+  return 0;
+}
+
+void
+tofu_begin_batch_update (void)
+{
+  batch_update ++;
+}
+
+void
+tofu_end_batch_update (void)
+{
+  assert (batch_update > 0);
+  batch_update --;
+
+  if (batch_update == 0)
+    {
+      struct db *db;
+
+      for (db = db_cache; db; db = db->next)
+        end_transaction (db, 1);
+    }
+}
+\f
 /* Collect results of a select count (*) ...; style query.  Aborts if
    the argument is not a valid integer (or real of the form X.0).  */
 static int
@@ -652,7 +825,7 @@ initdb (sqlite3 *db, enum db_type type)
     }
   else
     {
-      rc = sqlite3_exec (db, "commit transaction;", NULL, NULL, &err);
+      rc = sqlite3_exec (db, "end transaction;", NULL, NULL, &err);
       if (rc)
        {
          log_error (_("error committing transaction on TOFU DB: %s\n"),
@@ -736,10 +909,6 @@ link_db (struct db **head, struct db *db)
   *head = db;
 }
 
-static struct db *db_cache;
-static int db_cache_count;
-#define DB_CACHE_ENTRIES 16
-
 /* Return a database handle.  <type, name> describes the required
    database.  If there is a cached handle in DBS, that handle is
    returned.  Otherwise, the database is opened and cached in DBS.
@@ -889,7 +1058,10 @@ closedb (struct db *db)
       assert (db->name[0]);
     }
 
-  for (statements = &db->s.begin_transaction;
+  if (db->batch_update)
+    end_transaction (db, 2);
+
+  for (statements = (void *) &db->s;
        (void *) statements < (void *) &(&db->s)[1];
        statements ++)
     sqlite3_finalize (*statements);
@@ -983,7 +1155,14 @@ closedbs (struct dbs *dbs)
 
       /* Find the last DB.  */
       for (db = dbs->db, count = 1; db->next; db = db->next, count ++)
-        ;
+        {
+          /* When we leave batch mode we leave batch mode on any
+             cached connections.  */
+          if (! batch_update)
+            assert (! db->batch_update);
+        }
+      if (! batch_update)
+        assert (! db->batch_update);
 
       /* Join the two lists.  */
       db->next = db_cache;
@@ -1084,29 +1263,22 @@ record_binding (struct dbs *dbs, const char *fingerprint, const char *email,
       if (! db_key)
        return gpg_error (GPG_ERR_GENERAL);
 
-      rc = sqlite3_stepx (db_email->db, &db_email->s.begin_transaction,
-                          NULL, NULL, &err,
-                          "begin transaction;", SQLITE_ARG_END);
+      rc = begin_transaction (db_email, 0);
       if (rc)
-       {
-         log_error (_("error beginning transaction on TOFU %s database: %s\n"),
-                    "email", err);
-         sqlite3_free (err);
-         return gpg_error (GPG_ERR_GENERAL);
-       }
+        return gpg_error (GPG_ERR_GENERAL);
 
-      rc = sqlite3_stepx (db_key->db, &db_key->s.begin_transaction,
-                          NULL, NULL, &err,
-                          "begin transaction;", SQLITE_ARG_END);
+      rc = begin_transaction (db_key, 0);
       if (rc)
-       {
-         log_error (_("error beginning transaction on TOFU %s database: %s\n"),
-                    "key", err);
-         sqlite3_free (err);
-         goto out_revert_one;
-       }
+        goto out_revert_one;
+    }
+  else
+    {
+      rc = begin_transaction (db_email, 1);
+      if (rc)
+        return gpg_error (GPG_ERR_GENERAL);
     }
 
+
   if (show_old)
     /* Get the old policy.  Since this is just for informational
        purposes, there is no need to start a transaction or to die if
@@ -1206,13 +1378,9 @@ record_binding (struct dbs *dbs, const char *fingerprint, const char *email,
       int rc2;
 
       if (rc)
-        rc2 = sqlite3_stepx (db_key->db, &db_key->s.rollback,
-                             NULL, NULL, &err,
-                             "rollback;", SQLITE_ARG_END);
+        rc2 = rollback_transaction (db_key);
       else
-        rc2 = sqlite3_stepx (db_key->db, &db_key->s.end_transaction,
-                             NULL, NULL, &err,
-                             "end transaction;", SQLITE_ARG_END);
+        rc2 = end_transaction (db_key, 0);
       if (rc2)
        {
          log_error (_("error ending transaction on TOFU database: %s\n"),
@@ -1222,13 +1390,9 @@ record_binding (struct dbs *dbs, const char *fingerprint, const char *email,
 
     out_revert_one:
       if (rc)
-        rc2 = sqlite3_stepx (db_email->db, &db_email->s.rollback,
-                             NULL, NULL, &err,
-                             "rollback;", SQLITE_ARG_END);
+        rc2 = rollback_transaction (db_email);
       else
-        rc2 = sqlite3_stepx (db_email->db, &db_email->s.end_transaction,
-                             NULL, NULL, &err,
-                             "end transaction;", SQLITE_ARG_END);
+        rc2 = end_transaction (db_email, 0);
       if (rc2)
        {
          log_error (_("error ending transaction on TOFU database: %s\n"),
@@ -2524,15 +2688,9 @@ tofu_register (const byte *fingerprint_bin, const char *user_id,
 
   /* We do a query and then an insert.  Make sure they are atomic
      by wrapping them in a transaction.  */
-  rc = sqlite3_stepx (db->db, &db->s.begin_transaction,
-                      NULL, NULL, &err, "begin transaction;",
-                      SQLITE_ARG_END);
+  rc = begin_transaction (db, 0);
   if (rc)
-    {
-      log_error (_("error beginning transaction on TOFU database: %s\n"), err);
-      sqlite3_free (err);
-      goto die;
-    }
+    goto die;
 
   /* If we've already seen this signature before, then don't add
      it again.  */
@@ -2607,11 +2765,9 @@ tofu_register (const byte *fingerprint_bin, const char *user_id,
   /* It only matters whether we abort or commit the transaction
      (so long as we do something) if we execute the insert.  */
   if (rc)
-    rc = sqlite3_stepx (db->db, &db->s.rollback, NULL, NULL, &err,
-                        "rollback;", SQLITE_ARG_END);
+    rc = rollback_transaction (db);
   else
-    rc = sqlite3_stepx (db->db, &db->s.end_transaction, NULL, NULL, &err,
-                        "end transaction;", SQLITE_ARG_END);
+    rc = end_transaction (db, 0);
   if (rc)
     {
       log_error (_("error ending transaction on TOFU database: %s\n"), err);
index adf87ab..2d23e86 100644 (file)
@@ -106,4 +106,10 @@ gpg_error_t tofu_set_policy_by_keyid (u32 *keyid, enum tofu_policy policy);
 gpg_error_t tofu_get_policy (PKT_public_key *pk, PKT_user_id *user_id,
                             enum tofu_policy *policy);
 
+/* When doing a lot of DB activities (in particular, when listing
+   keys), this causes the DB to enter batch mode, which can
+   significantly speed up operations.  */
+void tofu_begin_batch_update (void);
+void tofu_end_batch_update (void);
+
 #endif /*G10_TOFU_H*/