git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: "Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com>
Cc: git@vger.kernel.org, sbeller@google.com, peff@peff.net,
	jrnieder@gmail.com, avarab@gmail.com, jonathantanmy@google.com,
	Derrick Stolee <dstolee@microsoft.com>
Subject: Re: [PATCH 06/11] multi-pack-index: implement 'expire' subcommand
Date: Tue, 11 Jun 2019 11:51:34 -0700	[thread overview]
Message-ID: <xmqqo934klah.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <8213541052a7d040aad4352ee981c0de01cc6516.1560209720.git.gitgitgadget@gmail.com> (Derrick Stolee via GitGitGadget's message of "Mon, 10 Jun 2019 16:35:25 -0700 (PDT)")

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <dstolee@microsoft.com>
>
> The 'git multi-pack-index expire' subcommand looks at the existing
> mult-pack-index, counts the number of objects referenced in each

s/mult/&i/

> pack-file, deletes the pack-fils with no referenced objects, and

s/fils/files/

> rewrites the multi-pack-index to no longer reference those packs.

As we do *not* want to expire a packfile that was created after the
last run of "git multi-pack-index write", "... in each pack-file" in
the above paragraph is a bit tricky.  It cannot literally be "in
each packfile", but a subset of all existing packfiles that are
known to the midx we are looking at.

An obvious alternative is to actually update midx file (so it no
longer "looks at the existing multi-pack-index", but "makes sure we
have up-to-date view of the multi-pack-index that covers all packs).

And I actually suspect that you implemented that alternative here.
IOW, "looks at the existing multi-pack-index" in the first paragraph
is misleading.

I wondered if the error behaviour of add_pack_to_midx() that ignores
a pack that cannot be added to midx for whatever reason can cause
issues, but such a pack would not become part of m->packs[] so it
may not have anything count[]ed from it, but it will not be eligible
for expiration, either.  So we'd be safe there, I think.

> Test that a new pack-file that covers the contents of two other
> pack-files leads to those pack-files being deleted during the
> expire subcommand. Be sure to read the multi-pack-index to ensure
> it no longer references those packs.

I am curious to learn how we guarantee that the midx logic chooses
to record the copy of an object in the new single packfile over the
other copy of the same object in one of the two older and presumably
smaller packfiles for all objects in the latter two packfiles,
because they cannot be expired if even one object were used from
there.

  reply	other threads:[~2019-06-11 18:51 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2019-06-10 23:35 [PATCH 00/11] Create 'expire' and 'repack' verbs for git-multi-pack-index Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 01/11] repack: refactor pack deletion for future use Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 02/11] Docs: rearrange subcommands for multi-pack-index Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 03/11] multi-pack-index: prepare for 'expire' subcommand Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 04/11] midx: simplify computation of pack name lengths Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 05/11] midx: refactor permutation logic and pack sorting Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 06/11] multi-pack-index: implement 'expire' subcommand Derrick Stolee via GitGitGadget
2019-06-11 18:51   ` Junio C Hamano [this message]
2019-06-10 23:35 ` [PATCH 07/11] multi-pack-index: prepare 'repack' subcommand Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 08/11] midx: implement midx_repack() Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 09/11] multi-pack-index: test expire while adding packs Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 10/11] midx: add test that 'expire' respects .keep files Derrick Stolee via GitGitGadget
2019-06-10 23:35 ` [PATCH 11/11] t5319-multi-pack-index.sh: test batch size zero Derrick Stolee via GitGitGadget
2019-06-30 18:57 ` [PATCH] t5319: don't trip over a user name with whitespace Johannes Sixt
2019-06-30 19:48   ` Eric Sunshine
2019-06-30 20:59     ` Johannes Sixt
2019-06-30 22:25       ` Jeff King
2019-07-01  6:33         ` Johannes Sixt
2019-07-01  9:16           ` Jeff King
2019-07-01 11:33             ` SZEDER Gábor
2019-07-01 12:03               ` Derrick Stolee
2019-07-01 12:11         ` Johannes Schindelin
2019-07-01 12:30           ` Derrick Stolee
2019-07-01 18:22             ` Johannes Sixt
2019-07-01 18:47               ` Derrick Stolee
2019-07-01 12:53           ` Jeff King
2019-07-01  8:36       ` SZEDER Gábor
2019-07-01 17:17   ` Andreas Schwab
2019-07-01 19:24     ` Junio C Hamano

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=xmqqo934klah.fsf@gitster-ct.c.googlers.com \
    --to=gitster@pobox.com \
    --cc=avarab@gmail.com \
    --cc=dstolee@microsoft.com \
    --cc=git@vger.kernel.org \
    --cc=gitgitgadget@gmail.com \
    --cc=jonathantanmy@google.com \
    --cc=jrnieder@gmail.com \
    --cc=peff@peff.net \
    --cc=sbeller@google.com \
    /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).