Re: [PATCH v5 9/9] odb/transaction: add transaction interface to write packfiles

Justin Tobler <[email protected]> writes:

> In git-receive-pack(1), the incoming packfile is written to the ODB via
> `unpack()`, which spawns git-index-pack(1) or git-unpack-objects(1)
> directly. With pluggable object databases, an alternative backend may
> need to handle writing packfile data differently though.
>
> Introduce `odb_transaction_write_pack()` as a generic interface to
> handle writing a packfile to a transaction and use the logic from
> `unpack()` as the "files" backend implementation. Note that when storing
> the objects as a packfile, git-index-pack(1) also writes a ".keep"
> lockfile next to it to prevent a concurrent repack from removing the new
> pack prior to reference updates being performed. The "files" transaction
> backend is responsible for managing these ".keep" files and removes them
> post-commit once the transaction is finalized.
>
> Call sites in git-receive-pack(1) are updated accordingly.
>
> Signed-off-by: Justin Tobler <[email protected]>
> ---
>  builtin/receive-pack.c | 160 +-----------------------------------
>  object-file.c          | 178 +++++++++++++++++++++++++++++++++++++++++
>  odb/transaction.c      |   7 ++
>  odb/transaction.h      |  62 ++++++++++++++
>  4 files changed, 250 insertions(+), 157 deletions(-)

Reading receive.unpackLimit and transfer.unpackLimit in generic
object-layer code feels like a layering violation, as these settings
belong to the transfer layer.  However, deciding whether to unpack
or index is inherently up to the file-backend, which is what the
'*.unpacklimit' settings control.  Future ODB backends might not
distinguish loose from packed objects, and even if they do, their
performance characteristics will differ.

We can attribute these '*.unpackLimit' names to historical wart; we
lacked non-file ODB backends when they were named.  Had we named
them today, something like 'odb-file.unpackLimit' would have been
more accurate.  If we had other bulk-import mechanisms that use pack
streams, they would use the same '*.unpacklimit' to optimize the
object layout for file-backed ODB stores.

Thanks.