bulk-checkin: remove ODB transaction nesting

ODB transactions support being nested. Only the outermost
{begin,end}_odb_transaction() start and finish a transaction. This
allows internal object write codepaths to be optimized with ODB
transactions without worrying about whether a transaction is already
active. When {begin,end}_odb_transaction() is invoked during an active
transaction, these operations are essentially treated as no-ops. This
can make the interface a bit awkward to use, as calling
end_odb_transaction() does not guarantee that a transaction is actually
ended. Thus, in situations where a transaction needs to be explicitly
flushed, flush_odb_transaction() must be used.

To remove the need for an explicit transaction flush operation via
flush_odb_transaction() and better clarify transaction semantics, drop
the transaction nesting mechanism in favor of begin_odb_transaction()
returning a NULL transaction value to signal it was a no-op, and
end_odb_transaction() behaving as a no-op when a NULL transaction value
is passed. This is safe for existing callers as the transaction value
wired to end_odb_transaction() already comes from
begin_odb_transaction() and thus continues the same no-op behavior when
a transaction is already pending. With this model, passing a pending
transaction to end_odb_transaction() ensures it is committed at that
point in time.

Signed-off-by: Justin Tobler <jltobler@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 124c49306769a5bc0fa568dc606ce5eb7370bf7e..eb6ef704c3c75877e6d2551e13a4f164169f7e36 100644 (file)
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -33,7 +33,6 @@ struct bulk_checkin_packfile {
 struct odb_transaction {
        struct object_database *odb;
 
-       int nesting;
        struct tmp_objdir *objdir;
        struct bulk_checkin_packfile packfile;
 };
@@ -368,12 +367,11 @@ void fsync_loose_object_bulk_checkin(struct odb_transaction *transaction,
 
 struct odb_transaction *begin_odb_transaction(struct object_database *odb)
 {
-       if (!odb->transaction) {
-               CALLOC_ARRAY(odb->transaction, 1);
-               odb->transaction->odb = odb;
-       }
+       if (odb->transaction)
+               return NULL;
 
-       odb->transaction->nesting += 1;
+       CALLOC_ARRAY(odb->transaction, 1);
+       odb->transaction->odb = odb;
 
        return odb->transaction;
 }
@@ -389,14 +387,14 @@ void flush_odb_transaction(struct odb_transaction *transaction)
 
 void end_odb_transaction(struct odb_transaction *transaction)
 {
-       if (!transaction || transaction->nesting == 0)
-               BUG("Unbalanced ODB transaction nesting");
-
-       transaction->nesting -= 1;
-
-       if (transaction->nesting)
+       if (!transaction)
                return;
 
+       /*
+        * Ensure the transaction ending matches the pending transaction.
+        */
+       ASSERT(transaction == transaction->odb->transaction);
+
        flush_odb_transaction(transaction);
        transaction->odb->transaction = NULL;
        free(transaction);

diff --git a/bulk-checkin.h b/bulk-checkin.h
index ac8887f476b496ba11a36e325bf34277b6a2c476..51d0ac6134e79e769677bcab10fe75e394d5f55a 100644 (file)
--- a/bulk-checkin.h
+++ b/bulk-checkin.h
@@ -38,9 +38,8 @@ int index_blob_bulk_checkin(struct odb_transaction *transaction,
 /*
  * Tell the object database to optimize for adding
  * multiple objects. end_odb_transaction must be called
- * to make new objects visible. Transactions can be nested,
- * and objects are only visible after the outermost transaction
- * is complete or the transaction is flushed.
+ * to make new objects visible. If a transaction is already
+ * pending, NULL is returned.
  */
 struct odb_transaction *begin_odb_transaction(struct object_database *odb);
 
@@ -53,8 +52,7 @@ void flush_odb_transaction(struct odb_transaction *transaction);
 
 /*
  * Tell the object database to make any objects from the
- * current transaction visible if this is the final nested
- * transaction.
+ * current transaction visible.
  */
 void end_odb_transaction(struct odb_transaction *transaction);
 

diff --git a/object-file.c b/object-file.c
index bc15af424509490720c399b7f29fc19e7867a65c..5e7657354953100dd00d5030379eaaec6e5844fc 100644 (file)
--- a/object-file.c
+++ b/object-file.c
@@ -1267,7 +1267,7 @@ int index_fd(struct index_state *istate, struct object_id *oid,
                struct odb_transaction *transaction;
 
                transaction = begin_odb_transaction(the_repository->objects);
-               ret = index_blob_bulk_checkin(transaction,
+               ret = index_blob_bulk_checkin(the_repository->objects->transaction,
                                              oid, fd, xsize_t(st->st_size),
                                              path, flags);
                end_odb_transaction(transaction);