[Pkg-gnupg-commit] [gnupg2] 23/118: g10: Improve TOFU batch update code.
Daniel Kahn Gillmor
dkg at fifthhorseman.net
Thu Sep 15 18:25:00 UTC 2016
This is an automated email from the git hooks/post-receive script.
dkg pushed a commit to branch encoding-and-speling
in repository gnupg2.
commit 371ae66e9d5c7524431773c4a479fcae1ea3b652
Author: Neal H. Walfield <neal at g10code.com>
Date: Tue Aug 30 15:37:45 2016 +0200
g10: Improve TOFU batch update code.
* g10/gpg.h (tofu): Rename field batch_update_ref to
batch_updated_wanted.
* g10/tofu.c (struct tofu_dbs_s): Rename field batch_update to
in_batch_transaction.
(begin_transaction): Only end an extant batch transaction if we are
not in a normal transaction. When ending a batch transaction, really
end it. Update ctrl->tofu.batch_update_started when starting a batch
transaction.
(end_transaction): Only release a batch transaction if ONLY_BATCH is
true. When releasing a batch transaction, assert that there is no
open normal transaction. Only allow DBS to be NULL if ONLY_BATCH is
true.
(tofu_begin_batch_update): Don't update
ctrl->tofu.batch_update_started.
(opendbs): Call end_transaction unconditionally.
--
Signed-off-by: Neal H. Walfield <neal at g10code.com>
---
g10/gpg.h | 2 +-
g10/tofu.c | 108 +++++++++++++++++++++++++++++++++++--------------------------
2 files changed, 64 insertions(+), 46 deletions(-)
diff --git a/g10/gpg.h b/g10/gpg.h
index 154da0d..33a3af6 100644
--- a/g10/gpg.h
+++ b/g10/gpg.h
@@ -83,7 +83,7 @@ struct server_control_s
struct {
tofu_dbs_t dbs;
int in_transaction;
- int batch_update_ref;
+ int batch_updated_wanted;
time_t batch_update_started;
} tofu;
diff --git a/g10/tofu.c b/g10/tofu.c
index 338fb3e..f84609e 100644
--- a/g10/tofu.c
+++ b/g10/tofu.c
@@ -81,7 +81,7 @@ struct tofu_dbs_s
sqlite3_stmt *register_insert;
} s;
- int batch_update;
+ int in_batch_transaction;
};
@@ -169,26 +169,39 @@ begin_transaction (ctrl_t ctrl, int only_batch)
log_assert (dbs);
- if (ctrl->tofu.batch_update_ref
+ /* If we've been in batch update mode for a while (on average, more
+ * than 500 ms), to prevent starving other gpg processes, we drop
+ * and retake the batch lock.
+ *
+ * Note: if we wanted higher resolution, we could use
+ * npth_clock_gettime. */
+ if (/* No real transactions. */
+ ctrl->tofu.in_transaction == 0
+ /* There is an open batch transaction. */
+ && dbs->in_batch_transaction
+ /* And some time has gone by since it was started. */
&& ctrl->tofu.batch_update_started != gnupg_get_time ())
{
- /* We've been in batch update mode for a while (on average, more
- * than 500 ms). To prevent starving other gpg processes, we
- * drop and retake the batch lock.
- *
- * Note: if we wanted higher resolution, we could use
- * npth_clock_gettime. */
- if (dbs->batch_update)
- end_transaction (ctrl, 1);
+ /* If we are in a batch update, then batch updates better have
+ been enabled. */
+ log_assert (ctrl->tofu.batch_updated_wanted);
- ctrl->tofu.batch_update_started = gnupg_get_time ();
+ end_transaction (ctrl, 2);
/* Yield to allow another process a chance to run. */
gpgrt_yield ();
}
- if (ctrl->tofu.batch_update_ref && !dbs->batch_update)
+ if (/* Batch mode is enabled. */
+ ctrl->tofu.batch_updated_wanted
+ /* But we don't have an open batch transaction. */
+ && !dbs->in_batch_transaction)
{
+ /* We are in batch mode, but we don't have an open batch
+ * transaction. Since the batch save point must be the outer
+ * save point, it must be taken before the inner save point. */
+ log_assert (ctrl->tofu.in_transaction == 0);
+
rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch,
NULL, NULL, &err,
"savepoint batch;", SQLITE_ARG_END);
@@ -200,7 +213,8 @@ begin_transaction (ctrl_t ctrl, int only_batch)
return gpg_error (GPG_ERR_GENERAL);
}
- dbs->batch_update = 1;
+ dbs->in_batch_transaction = 1;
+ ctrl->tofu.batch_update_started = gnupg_get_time ();
}
if (only_batch)
@@ -235,35 +249,44 @@ end_transaction (ctrl_t ctrl, int only_batch)
int rc;
char *err = NULL;
- if (!dbs)
- return 0; /* Shortcut to allow for easier cleanup code. */
-
- if ((!ctrl->tofu.batch_update_ref || only_batch == 2) && dbs->batch_update)
+ if (only_batch)
{
- /* The batch transaction is still in open, but we left batch
- * mode. */
- dbs->batch_update = 0;
+ if (!dbs)
+ return 0; /* Shortcut to allow for easier cleanup code. */
- rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch_commit,
- NULL, NULL, &err,
- "release batch;", SQLITE_ARG_END);
- if (rc)
+ /* If we are releasing the batch transaction, then we better not
+ be in a normal transaction. */
+ log_assert (ctrl->tofu.in_transaction == 0);
+
+ if (/* Batch mode disabled? */
+ (!ctrl->tofu.batch_updated_wanted || only_batch == 2)
+ /* But, we still have an open batch transaction? */
+ && dbs->in_batch_transaction)
{
- log_error (_("error committing transaction on TOFU database: %s\n"),
- err);
- sqlite3_free (err);
- return gpg_error (GPG_ERR_GENERAL);
+ /* The batch transaction is still in open, but we've left
+ * batch mode. */
+ dbs->in_batch_transaction = 0;
+
+ rc = gpgsql_stepx (dbs->db, &dbs->s.savepoint_batch_commit,
+ NULL, NULL, &err,
+ "release batch;", SQLITE_ARG_END);
+ if (rc)
+ {
+ log_error (_("error committing transaction on TOFU database: %s\n"),
+ err);
+ sqlite3_free (err);
+ return gpg_error (GPG_ERR_GENERAL);
+ }
+
+ return 0;
}
- /* Releasing an outer transaction releases an open inner
- transactions. We're done. */
return 0;
}
- if (only_batch)
- return 0;
-
+ log_assert (dbs);
log_assert (ctrl->tofu.in_transaction > 0);
+
rc = gpgsql_exec_printf (dbs->db, NULL, NULL, &err,
"release inner%d;", ctrl->tofu.in_transaction);
if (rc)
@@ -287,8 +310,7 @@ rollback_transaction (ctrl_t ctrl)
int rc;
char *err = NULL;
- if (!dbs)
- return 0; /* Shortcut to allow for easier cleanup code. */
+ log_assert (dbs);
log_assert (ctrl->tofu.in_transaction > 0);
/* Be careful to not any progress made by closed transactions in
@@ -313,19 +335,16 @@ rollback_transaction (ctrl_t ctrl)
void
tofu_begin_batch_update (ctrl_t ctrl)
{
- if (!ctrl->tofu.batch_update_ref)
- ctrl->tofu.batch_update_started = gnupg_get_time ();
-
- ctrl->tofu.batch_update_ref ++;
+ ctrl->tofu.batch_updated_wanted ++;
}
void
tofu_end_batch_update (ctrl_t ctrl)
{
- log_assert (ctrl->tofu.batch_update_ref > 0);
- ctrl->tofu.batch_update_ref --;
+ log_assert (ctrl->tofu.batch_updated_wanted > 0);
+ ctrl->tofu.batch_updated_wanted --;
- if (!ctrl->tofu.batch_update_ref)
+ if (!ctrl->tofu.batch_updated_wanted)
end_transaction (ctrl, 1);
}
@@ -693,14 +712,13 @@ tofu_closedbs (ctrl_t ctrl)
tofu_dbs_t dbs;
sqlite3_stmt **statements;
- log_assert(ctrl->tofu.in_transaction == 0);
+ log_assert (ctrl->tofu.in_transaction == 0);
dbs = ctrl->tofu.dbs;
if (!dbs)
return; /* Not initialized. */
- if (dbs->batch_update)
- end_transaction (ctrl, 2);
+ end_transaction (ctrl, 2);
/* Arghh, that is a surprising use of the struct. */
for (statements = (void *) &dbs->s;
--
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-gnupg/gnupg2.git
More information about the Pkg-gnupg-commit
mailing list