public inbox for git@vger.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Patrick Steinhardt <ps@pks.im>
Cc: Justin Tobler <jltobler@gmail.com>,  git@vger.kernel.org
Subject: Re: [PATCH 1/4] odb: store ODB source in `struct odb_transaction`
Date: Thu, 29 Jan 2026 11:25:48 -0800	[thread overview]
Message-ID: <xmqqcy2sb4qr.fsf@gitster.g> (raw)
In-Reply-To: <aXtDYY0Ao24Mpgyb@pks.im> (Patrick Steinhardt's message of "Thu, 29 Jan 2026 12:24:17 +0100")

Patrick Steinhardt <ps@pks.im> writes:

> On Wed, Jan 28, 2026 at 05:45:16PM -0600, Justin Tobler wrote:
>> Each `struct odb_transaction` currently stores a reference to the
>> `struct object_database`. Since transactions are handled per object
>> source, instead store a reference to the source.
>
> Makes sense.
>
>> diff --git a/object-file.c b/object-file.c
>> index e7e4c3348f..196509b252 100644
>> --- a/object-file.c
>> +++ b/object-file.c
>> @@ -728,7 +728,7 @@ static void prepare_loose_object_transaction(struct odb_transaction *transaction
>>  	if (!transaction || transaction->objdir)
>>  		return;
>>  
>> -	transaction->objdir = tmp_objdir_create(transaction->odb->repo, "bulk-fsync");
>> +	transaction->objdir = tmp_objdir_create(transaction->source->odb->repo, "bulk-fsync");
>>  	if (transaction->objdir)
>>  		tmp_objdir_replace_primary_odb(transaction->objdir, 0);
>>  }
>
> This makes me wonder whether we should first refactor the `tmp_objdir`
> subsystem to receive a source instead of a repository as input.
> Otherwise we "pretend" that the transaction is on the source level, but
> we ultimately still end up creating the temporary directory in the
> repository's object directory unconditionally.
>
> It wouldn't really change anything right now as we only ever write
> objects via the primary object source anyway, so the end result would be
> the same. But it just feels like a good first step to me to fix this
> conceptual inconsistency, and it shouldn't be too involved either as
> `tmp_objdir_create()` only has three callsites.

I agree with your "not really change anything right now" comment,
but a new odb source that will be invented in the future may not
even be file based, and a generic-sounding tmp_objdir_create() that
creates a temporary directory on the filesystem may not even be an
appropriate abstraction.

If we have two or more odb sources both are filesystem based, on the
other hand, I do not think it is particulary bad if these two odb
sources belonging to the same repository took a temporary directory
out of that repository.  As long as one temporary object directory
taken by one odb source is not used to commit the transaction into
the other odb source, it would be fine, no?

Thanks.



  reply	other threads:[~2026-01-29 19:26 UTC|newest]

Thread overview: 32+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-01-28 23:45 [PATCH 0/4] odb: support ODB source specific transaction handling Justin Tobler
2026-01-28 23:45 ` [PATCH 1/4] odb: store ODB source in `struct odb_transaction` Justin Tobler
2026-01-29 11:24   ` Patrick Steinhardt
2026-01-29 19:25     ` Junio C Hamano [this message]
2026-01-29 20:12       ` Justin Tobler
2026-01-29 20:28         ` Junio C Hamano
2026-01-29 21:54           ` Justin Tobler
2026-01-29 19:30     ` Justin Tobler
2026-01-28 23:45 ` [PATCH 2/4] object-file: rename transaction functions Justin Tobler
2026-01-28 23:45 ` [PATCH 3/4] odb: prepare `struct odb_transaction` to support more sources Justin Tobler
2026-01-29 11:24   ` Patrick Steinhardt
2026-01-29 19:41     ` Justin Tobler
2026-01-28 23:45 ` [PATCH 4/4] odb: transparently handle common transaction behavior Justin Tobler
2026-01-29 11:24   ` Patrick Steinhardt
2026-02-03  0:09 ` [PATCH v2 0/4] odb: support ODB source specific transaction handling Justin Tobler
2026-02-03  0:09   ` [PATCH v2 1/4] odb: store ODB source in `struct odb_transaction` Justin Tobler
2026-02-03  0:10   ` [PATCH v2 2/4] object-file: rename transaction functions Justin Tobler
2026-02-03  0:10   ` [PATCH v2 3/4] odb: prepare `struct odb_transaction` to become generic Justin Tobler
2026-02-03 15:54     ` Toon Claes
2026-02-03 16:46       ` Justin Tobler
2026-02-03 22:54         ` Junio C Hamano
2026-02-04  6:26           ` Patrick Steinhardt
2026-02-04 17:15             ` Justin Tobler
2026-02-04 10:31     ` Karthik Nayak
2026-02-04 17:38       ` Justin Tobler
2026-02-05 11:20         ` Karthik Nayak
2026-02-03  0:10   ` [PATCH v2 4/4] odb: transparently handle common transaction behavior Justin Tobler
2026-02-04 10:34     ` Karthik Nayak
2026-02-04 17:50       ` Justin Tobler
2026-02-05 11:22         ` Karthik Nayak
2026-02-03  1:16   ` [PATCH v2 0/4] odb: support ODB source specific transaction handling Junio C Hamano
2026-02-04  6:25     ` Patrick Steinhardt

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqcy2sb4qr.fsf@gitster.g \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=jltobler@gmail.com \
    --cc=ps@pks.im \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox