git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Jacob Keller <jacob.keller@gmail.com>
To: David Turner <dturner@twosigma.com>
Cc: Git mailing list <git@vger.kernel.org>,
	Christian Couder <christian.couder@gmail.com>,
	Martin Fick <mfick@codeaurora.org>
Subject: Re: [PATCH] repack: respect gc.pid lock
Date: Thu, 13 Apr 2017 17:33:55 -0700	[thread overview]
Message-ID: <CA+P7+xomqvK=E0A_wPiyufzyF63yRLA=CQS3Sfec_Uub72DKrw@mail.gmail.com> (raw)
In-Reply-To: <20170413202712.22192-1-dturner@twosigma.com>

On Thu, Apr 13, 2017 at 1:27 PM, David Turner <dturner@twosigma.com> wrote:
> Git gc locks the repository (using a gc.pid file) so that other gcs
> don't run concurrently. Make git repack respect this lock.
>
> Now repack, by default, will refuse to run at the same time as a gc.
> This fixes a concurrency issue: a repack which deleted packs would
> make a concurrent gc sad when its packs were deleted out from under
> it.  The gc would fail with: "fatal: ./objects/pack/pack-$sha.pack
> cannot be accessed".  Then it would die, probably leaving a large temp
> pack hanging around.
>
> Git repack learns --no-lock, so that when run under git gc, it doesn't
> attempt to manage the lock itself.
>
> Martin Fick suggested just moving the lock into git repack, but this
> would leave parts of git gc (e.g. git prune) protected by only local
> locks.  I worried that a prune (part of git gc) concurrent with a
> repack could confuse the repack, so I decided to go with this
> solution.
>

The last paragraph could be reworded to be a bit less personal and
more as a direct statement of why moving the lock entirely to repack
is a bad idea.

> Signed-off-by: David Turner <dturner@twosigma.com>
> ---
>  Documentation/git-repack.txt |  5 +++
>  Makefile                     |  1 +
>  builtin/gc.c                 | 72 ++----------------------------------
>  builtin/repack.c             | 13 +++++++
>  repack.c                     | 88 ++++++++++++++++++++++++++++++++++++++++++++
>  repack.h                     |  8 ++++
>  t/t7700-repack.sh            |  8 ++++
>  7 files changed, 127 insertions(+), 68 deletions(-)
>  create mode 100644 repack.c
>  create mode 100644 repack.h
>
> diff --git a/Documentation/git-repack.txt b/Documentation/git-repack.txt
> index 26afe6ed54..b347ff5c62 100644
> --- a/Documentation/git-repack.txt
> +++ b/Documentation/git-repack.txt
> @@ -143,6 +143,11 @@ other objects in that pack they already have locally.
>         being removed. In addition, any unreachable loose objects will
>         be packed (and their loose counterparts removed).
>
> +--no-lock::
> +       Do not lock the repository, and do not respect any existing lock.
> +       Mostly useful for running repack within git gc.  Do not use this
> +       unless you know what you are doing.
> +

I would have phrased this more like:

  Used internally by git gc to call git repack while already holding
the lock. Do not use unless you know what you're doing.

  reply	other threads:[~2017-04-14  0:34 UTC|newest]

Thread overview: 13+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-04-13 20:27 [PATCH] repack: respect gc.pid lock David Turner
2017-04-14  0:33 ` Jacob Keller [this message]
2017-04-14 19:33 ` Jeff King
2017-04-17 23:29   ` David Turner
2017-04-18  3:41     ` Jeff King
2017-04-18 17:08       ` David Turner
2017-04-18 17:16         ` Jeff King
2017-04-18 17:16       ` David Turner
2017-04-18 17:19         ` Jeff King
2017-04-18 17:43           ` David Turner
2017-04-18 17:50             ` Jeff King
2017-04-20 20:10               ` David Turner
2017-04-20 20:14                 ` Jeff King

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

  List information: http://vger.kernel.org/majordomo-info.html

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

  git send-email \
    --in-reply-to='CA+P7+xomqvK=E0A_wPiyufzyF63yRLA=CQS3Sfec_Uub72DKrw@mail.gmail.com' \
    --to=jacob.keller@gmail.com \
    --cc=christian.couder@gmail.com \
    --cc=dturner@twosigma.com \
    --cc=git@vger.kernel.org \
    --cc=mfick@codeaurora.org \
    /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.
Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).