git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v3 0/6] odb: add transaction interfaces to ODB subsystem

From
Justin Tobler <jltobler@gmail.com>
Date
Sep 16, 2025, 18:29 UTC
Message-ID
<20250916182938.2193476-1-jltobler@gmail.com>
In-Reply-To
<20250915202956.3784935-1-jltobler@gmail.com>
Greetings,

This series is a followup to [1] and continues iterating on the ODB transaction interfaces.

The bulk-checkin subsystem provides an interface to manage ODB transactions. Apart from {begin,end}_odb_transaction(), these functions are only used by the object-file subsystem to manage aspects of a transaction implementation specific to the files object source.

In a pluggable object database future where we could have different types of object database sources, transaction handling will have to be implemented separately per source. Thus, the primary focus of this series is to simplify the existing ODB transaction interface and provide a means to manage transactions via the ODB subsystem in an object source agnostic manner eventually.

This series is built on top of 4975ec3473b (The seventh batch, 2025-09-08) with jt/de-global-bulk-checkin merged into it at ddc0b56ad77 (bulk-checkin: use repository variable from transaction, 2025-08-22).

Changes since V1:
- end_odb_transaction() is now a no-op when no transaction is specified
  and also guards against the ending a transaction not set in the object
  database.
- The object_file_transaction_begin() now accepts a `struct odb_source`
  instead of `struct odbject_database`. Which is more in line with it
  being a transaction implementation specific to an object source.
- Further clarified some commit messages.
- Renamed object_file_transaction_end() to
  object_file_transaction_commit() to match the corresponding function
  in the ODB section.
- Add some missing documentation for odb_transaction_{begin,commit}().
Change since V2:
- begin_odb_transaction() now behaves as a no-op and returns a NULL
  transaction value when the ODB already has an active transaction.
  Callers are no longer required to manually check if there is an active
  transaction beforehand.
- Reworded several commit messages.

Thanks, -Justin

[1]: <20250820225531.1212935-1-jltobler@gmail.com>
Justin Tobler (6):
  bulk-checkin: remove ODB transaction nesting
  builtin/update-index: end ODB transaction when --verbose is specified
  bulk-checkin: drop flush_odb_transaction()
  object-file: relocate ODB transaction code
  object-file: update naming from bulk-checkin
  odb: add transaction interface
 Makefile                 |   1 -
 builtin/add.c            |   7 +-
 builtin/unpack-objects.c |   5 +-
 builtin/update-index.c   |  29 +--
 bulk-checkin.c           | 403 --------------------------------------
 bulk-checkin.h           |  61 ------
 cache-tree.c             |   5 +-
 meson.build              |   1 -
 object-file.c            | 404 ++++++++++++++++++++++++++++++++++++++-
 object-file.h            |  16 ++
 odb.c                    |  10 +
 odb.h                    |  13 ++
 read-cache.c             |   5 +-
 13 files changed, 462 insertions(+), 498 deletions(-)
 delete mode 100644 bulk-checkin.c
 delete mode 100644 bulk-checkin.h
Range-diff against v2:
1:  217f3eb6d2 ! 1:  9fe2ab4198 bulk-checkin: remove ODB transaction nesting
    @@ Commit message
         bulk-checkin: remove ODB transaction nesting
     
         ODB transactions support being nested. Only the outermost
    -    {begin,end}_odb_transaction() start and finish a transaction. This is
    -    done so that certain object write codepaths that occur internally can be
    -    optimized via ODB transactions without having to worry if a transaction
    -    has already been started or not. This can make the interface a bit
    -    awkward to use, as calling {begin,end}_odb_transaction() does not
    -    guarantee that a transaction is actually started or ended. Thus, in
    -    situations where a transaction must be explicitly flushed,
    -    flush_odb_transaction() must be used.
    +    {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 better clarify ownership sematics around a transaction and further
    -    remove the need for flush_odb_transaction() as part of the transaction
    -    interface, instead be more explicit and require callers who use ODB
    -    transactions internally to ensure there is not already a pending
    -    transaction before beginning or ending a transaction.
    +    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>
     
    @@ bulk-checkin.c: void fsync_loose_object_bulk_checkin(struct odb_transaction *tra
     -		odb->transaction->odb = odb;
     -	}
     +	if (odb->transaction)
    -+		BUG("ODB transaction already started");
    ++		return NULL;
      
     -	odb->transaction->nesting += 1;
     +	CALLOC_ARRAY(odb->transaction, 1);
    @@ bulk-checkin.h: int index_blob_bulk_checkin(struct odb_transaction *transaction,
     - * 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. Only a single transaction
    -+ * can be pending at a time and must be ended before
    -+ * beginning another.
    ++ * to make new objects visible. If a transaction is already
    ++ * pending, NULL is returned.
       */
      struct odb_transaction *begin_odb_transaction(struct object_database *odb);
      
    @@ bulk-checkin.h: void flush_odb_transaction(struct odb_transaction *transaction);
      void end_odb_transaction(struct odb_transaction *transaction);
      
     
    - ## cache-tree.c ##
    -@@ cache-tree.c: static int update_one(struct cache_tree *it,
    - 
    - int cache_tree_update(struct index_state *istate, int flags)
    - {
    --	struct odb_transaction *transaction;
    -+	struct odb_transaction *transaction = NULL;
    - 	int skip, i;
    - 
    - 	i = verify_cache(istate, flags);
    -@@ cache-tree.c: int cache_tree_update(struct index_state *istate, int flags)
    - 
    - 	trace_performance_enter();
    - 	trace2_region_enter("cache_tree", "update", the_repository);
    --	transaction = begin_odb_transaction(the_repository->objects);
    -+
    -+	if (!the_repository->objects->transaction)
    -+		transaction = begin_odb_transaction(the_repository->objects);
    -+
    - 	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
    - 		       "", 0, &skip, flags);
    -+
    - 	end_odb_transaction(transaction);
    -+
    - 	trace2_region_leave("cache_tree", "update", the_repository);
    - 	trace_performance_leave("cache_tree_update");
    - 	if (i < 0)
    -
      ## object-file.c ##
     @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
    - 		ret = index_core(istate, oid, fd, xsize_t(st->st_size),
    - 				 type, path, flags);
    - 	} else {
    --		struct odb_transaction *transaction;
    -+		struct odb_transaction *transaction = NULL;
    + 		struct odb_transaction *transaction;
      
    --		transaction = begin_odb_transaction(the_repository->objects);
    + 		transaction = begin_odb_transaction(the_repository->objects);
     -		ret = index_blob_bulk_checkin(transaction,
    -+		if (!the_repository->objects->transaction)
    -+			transaction = begin_odb_transaction(the_repository->objects);
    -+
     +		ret = index_blob_bulk_checkin(the_repository->objects->transaction,
      					      oid, fd, xsize_t(st->st_size),
      					      path, flags);
    -+
      		end_odb_transaction(transaction);
    - 	}
    - 
    -
    - ## read-cache.c ##
    -@@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
    - 		       const struct pathspec *pathspec, char *ps_matched,
    - 		       int include_sparse, int flags)
    - {
    --	struct odb_transaction *transaction;
    -+	struct odb_transaction *transaction = NULL;
    - 	struct update_callback_data data;
    - 	struct rev_info rev;
    - 
    -@@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
    - 	 * This function is invoked from commands other than 'add', which
    - 	 * may not have their own transaction active.
    - 	 */
    --	transaction = begin_odb_transaction(repo->objects);
    -+	if (!repo->objects->transaction)
    -+		transaction = begin_odb_transaction(repo->objects);
    -+
    - 	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
    -+
    - 	end_odb_transaction(transaction);
    - 
    - 	release_revisions(&rev);
2:  16af9f1169 ! 2:  f24c28ec56 builtin/update-index: end ODB transaction when --verbose is specified
    @@ Commit message
         object directory and migrated to the primary object database upon
         transaction commit.
     
    -    When the --verbose option is specified, each of the following objects is
    -    explicitly flushed via flush_odb_transaction() prior to reporting the
    -    update. Flushing the object database transaction migrates pending
    -    objects to the primary object database without marking the transaction
    -    as complete. This is done so objects are immediately visible to
    -    git-update-index(1) callers using the --verbose option and that rely on
    -    parsing verbose output to know when objects are written.
    +    When the --verbose option is specified, the subsequent set of objects
    +    written are explicitly flushed via flush_odb_transaction() prior to
    +    reporting the update. Flushing the object database transaction migrates
    +    pending objects to the primary object database without marking the
    +    transaction as complete. This is done so objects are immediately visible
    +    to git-update-index(1) callers using the --verbose option and that rely
    +    on parsing verbose output to know when objects are written.
     
    -    Due to how git-update-index(1) parses options, each filename argument is
    -    evaluated with only the set of options that precede it. Therefore, it is
    -    possible for an initial set of objects to be written in a transaction
    -    before a --verbose option is encountered.
    +    Due to how git-update-index(1) parses arguments, options that come after
    +    a filename are not considered during the object update. Therefore, it
    +    may not be known ahead of time whether the --verbose option is present
    +    and thus object writes are considered transactional by default until a
    +    --verbose option is parsed.
     
    -    As soon as the --verbose option is parsed in git-update-index(1), all
    -    subsequent object writes are flushed prior to being reported and thus no
    -    longer benefit from being transactional. Furthermore, the mechanism to
    -    flush a transaction without committing is rather awkward. Drop the call
    -    to flush_odb_transaction() in favor of ending the transaction early when
    -    the --verbose flag is encountered.
    +    Flushing a transaction after individual object writes negates the
    +    benefit of writing objects to a transaction in the first place.
    +    Furthermore, the mechanism to flush a transaction without actually
    +    committing is rather awkward. Drop the call to flush_odb_transaction()
    +    in favor of ending the transaction altogether when the --verbose flag is
    +    encountered. Subsequent object writes occur outside of a transaction and
    +    are therefore immediately visible which matches the current behavior.
     
         Signed-off-by: Justin Tobler <jltobler@gmail.com>
     
3:  ae6199a3c8 = 3:  6268b2934a bulk-checkin: drop flush_odb_transaction()
4:  01f5485441 ! 4:  e5f1d93797 object-file: relocate ODB transaction code
    @@ Commit message
         are only used by the object-file subsystem to manage aspects of a
         transaction implementation specific to the files object source.
     
    -    Relocate all the transaction code in in bulk-checkin to object-file.
    -    This simplifies the exposed transaction interface by reducing it to only
    +    Relocate all the transaction code in bulk-checkin to object-file. This
    +    simplifies the exposed transaction interface by reducing it to only
         {begin,end}_odb_transaction(). Function and type names are adjusted in
         the subsequent commit to better fit the new location.
     
    @@ bulk-checkin.c (deleted)
     -struct odb_transaction *begin_odb_transaction(struct object_database *odb)
     -{
     -	if (odb->transaction)
    --		BUG("ODB transaction already started");
    +-		return NULL;
     -
     -	CALLOC_ARRAY(odb->transaction, 1);
     -	odb->transaction->odb = odb;
    @@ bulk-checkin.h (deleted)
     -/*
     - * Tell the object database to optimize for adding
     - * multiple objects. end_odb_transaction must be called
    -- * to make new objects visible. Only a single transaction
    -- * can be pending at a time and must be ended before
    -- * beginning another.
    +- * to make new objects visible. If a transaction is already
    +- * pending, NULL is returned.
     - */
     -struct odb_transaction *begin_odb_transaction(struct object_database *odb);
     -
    @@ object-file.c: int read_loose_object(struct repository *repo,
     +struct odb_transaction *begin_odb_transaction(struct object_database *odb)
     +{
     +	if (odb->transaction)
    -+		BUG("ODB transaction already started");
    ++		return NULL;
     +
     +	CALLOC_ARRAY(odb->transaction, 1);
     +	odb->transaction->odb = odb;
    @@ object-file.h: int read_loose_object(struct repository *repo,
     +/*
     + * Tell the object database to optimize for adding
     + * multiple objects. end_odb_transaction must be called
    -+ * to make new objects visible. Only a single transaction
    -+ * can be pending at a time and must be ended before
    -+ * beginning another.
    ++ * to make new objects visible. If a transaction is already
    ++ * pending, NULL is returned.
     + */
     +struct odb_transaction *begin_odb_transaction(struct object_database *odb);
     +
5:  333319a63d ! 5:  76eabfb6a1 object-file: update naming from bulk-checkin
    @@ object-file.c: static int index_blob_bulk_checkin(struct odb_transaction *transa
      			return error("cannot seek back");
      	}
     @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
    - 		if (!the_repository->objects->transaction)
    - 			transaction = begin_odb_transaction(the_repository->objects);
    + 		struct odb_transaction *transaction;
      
    + 		transaction = begin_odb_transaction(the_repository->objects);
     -		ret = index_blob_bulk_checkin(the_repository->objects->transaction,
     -					      oid, fd, xsize_t(st->st_size),
     -					      path, flags);
    @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
     +						      oid, fd,
     +						      xsize_t(st->st_size),
     +						      path, flags);
    - 
      		end_odb_transaction(transaction);
      	}
    + 
     @@ object-file.c: void end_odb_transaction(struct odb_transaction *transaction)
      	 */
      	ASSERT(transaction == transaction->odb->transaction);
6:  30759bbd0d ! 6:  9accbfdbf0 odb: add transaction interface
    @@ builtin/update-index.c: int cmd_update_index(int argc,
     
      ## cache-tree.c ##
     @@ cache-tree.c: int cache_tree_update(struct index_state *istate, int flags)
    - 	trace2_region_enter("cache_tree", "update", the_repository);
    - 
    - 	if (!the_repository->objects->transaction)
    --		transaction = begin_odb_transaction(the_repository->objects);
    -+		transaction = odb_transaction_begin(the_repository->objects);
      
    + 	trace_performance_enter();
    + 	trace2_region_enter("cache_tree", "update", the_repository);
    +-	transaction = begin_odb_transaction(the_repository->objects);
    ++	transaction = odb_transaction_begin(the_repository->objects);
      	i = update_one(istate->cache_tree, istate->cache, istate->cache_nr,
      		       "", 0, &skip, flags);
    - 
     -	end_odb_transaction(transaction);
     +	odb_transaction_commit(transaction);
    - 
      	trace2_region_leave("cache_tree", "update", the_repository);
      	trace_performance_leave("cache_tree_update");
    + 	if (i < 0)
     
      ## object-file.c ##
     @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transaction *transaction
    @@ object-file.c: static void prepare_loose_object_transaction(struct odb_transacti
      	if (!transaction || transaction->objdir)
      		return;
     @@ object-file.c: int index_fd(struct index_state *istate, struct object_id *oid,
    - 		struct odb_transaction *transaction = NULL;
    - 
    - 		if (!the_repository->objects->transaction)
    --			transaction = begin_odb_transaction(the_repository->objects);
    -+			transaction = odb_transaction_begin(the_repository->objects);
    + 	} else {
    + 		struct odb_transaction *transaction;
      
    +-		transaction = begin_odb_transaction(the_repository->objects);
    ++		transaction = odb_transaction_begin(the_repository->objects);
      		ret = index_blob_packfile_transaction(the_repository->objects->transaction,
      						      oid, fd,
      						      xsize_t(st->st_size),
      						      path, flags);
    - 
     -		end_odb_transaction(transaction);
     +		odb_transaction_commit(transaction);
      	}
    @@ object-file.c: int read_loose_object(struct repository *repo,
     +	struct object_database *odb = source->odb;
     +
      	if (odb->transaction)
    - 		BUG("ODB transaction already started");
    + 		return NULL;
      
     @@ object-file.c: struct odb_transaction *begin_odb_transaction(struct object_database *odb)
      	return odb->transaction;
    @@ object-file.h: struct odb_transaction;
       * Tell the object database to optimize for adding
     - * multiple objects. end_odb_transaction must be called
     + * multiple objects. object_file_transaction_commit must be called
    -  * to make new objects visible. Only a single transaction
    -  * can be pending at a time and must be ended before
    -  * beginning another.
    +  * to make new objects visible. If a transaction is already
    +  * pending, NULL is returned.
       */
     -struct odb_transaction *begin_odb_transaction(struct object_database *odb);
     +struct odb_transaction *object_file_transaction_begin(struct odb_source *source);
    @@ odb.h: struct object_database {
     +/*
     + * Starts an ODB transaction. Subsequent objects are written to the transaction
     + * and not committed until odb_transaction_commit() is invoked on the
    -+ * transaction. Caller are responsible to ensure there is only a single ODB
    -+ * transaction pending at a time.
    ++ * transaction. If the ODB already has a pending transaction, NULL is returned.
     + */
     +struct odb_transaction *odb_transaction_begin(struct object_database *odb);
     +
    @@ odb.h: struct object_database {
     
      ## read-cache.c ##
     @@ read-cache.c: int add_files_to_cache(struct repository *repo, const char *prefix,
    + 	 * This function is invoked from commands other than 'add', which
      	 * may not have their own transaction active.
      	 */
    - 	if (!repo->objects->transaction)
    --		transaction = begin_odb_transaction(repo->objects);
    -+		transaction = odb_transaction_begin(repo->objects);
    - 
    +-	transaction = begin_odb_transaction(repo->objects);
    ++	transaction = odb_transaction_begin(repo->objects);
      	run_diff_files(&rev, DIFF_RACY_IS_MODIFIED);
    - 
     -	end_odb_transaction(transaction);
     +	odb_transaction_commit(transaction);
      
-- 
2.51.0.193.g4975ec3473b
Previous: Justin ToblerNext: Justin Tobler
Message 32 of 38 in “odb: add transaction interfaces to ODB subsystem”
  1. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 9, 2025
  2. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 9, 2025
  3. Patrick SteinhardtSep 11, 2025
  4. Justin ToblerSep 11, 2025
  5. Taylor BlauSep 15, 2025
  6. Justin ToblerSep 16, 2025
  7. Junio C HamanoSep 16, 2025
  8. Justin ToblerSep 16, 2025
  9. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 9, 2025
  10. Patrick SteinhardtSep 11, 2025
  11. Justin ToblerSep 11, 2025
  12. Patrick SteinhardtSep 15, 2025
  13. Justin ToblerSep 15, 2025
  14. Justin ToblerSep 15, 2025
  15. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 9, 2025
  16. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 9, 2025
  17. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 9, 2025
  18. 6/6 odb: add transaction interfaceJustin Tobler, Sep 9, 2025
  19. Patrick SteinhardtSep 11, 2025
  20. Justin ToblerSep 11, 2025
  21. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 15, 2025
  22. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 15, 2025
  23. Karthik NayakSep 16, 2025
  24. Justin ToblerSep 16, 2025
  25. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 15, 2025
  26. Karthik NayakSep 16, 2025
  27. Justin ToblerSep 16, 2025
  28. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 15, 2025
  29. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 15, 2025
  30. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 15, 2025
  31. 6/6 odb: add transaction interfaceJustin Tobler, Sep 15, 2025
  32. 0/6 odb: add transaction interfaces to ODB subsystemJustin Tobler, Sep 16, 2025
  33. 2/6 builtin/update-index: end ODB transaction when --verbose is specifiedJustin Tobler, Sep 16, 2025
  34. 1/6 bulk-checkin: remove ODB transaction nestingJustin Tobler, Sep 16, 2025
  35. 3/6 bulk-checkin: drop flush_odb_transaction()Justin Tobler, Sep 16, 2025
  36. 4/6 object-file: relocate ODB transaction codeJustin Tobler, Sep 16, 2025
  37. 5/6 object-file: update naming from bulk-checkinJustin Tobler, Sep 16, 2025
  38. 6/6 odb: add transaction interfaceJustin Tobler, Sep 16, 2025

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.